Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tjson: Fix schema comparison #907

Closed
2 tasks done
Tracked by #683
rumyantseva opened this issue Jul 18, 2022 · 1 comment · Fixed by #944
Closed
2 tasks done
Tracked by #683

tjson: Fix schema comparison #907

rumyantseva opened this issue Jul 18, 2022 · 1 comment · Fixed by #944
Assignees
Labels
code/chore Code maintenance improvements
Milestone

Comments

@rumyantseva
Copy link
Member

rumyantseva commented Jul 18, 2022

This task is a part of #683 epic.

🎯 Currently, schema comparison method func (s *Schema) Equal(other *Schema) bool is implemented with reflect.DeepEqual. The goal of this task is to come up with a better approach.

The target code is located here.

Tigris' data types consist of type and format. For int64 and double format is not required but might be set.

Having that in mind, the following schemas must be equal:

	doubleSchema = &Schema{
		Type: Number,
	}
	doubleSchemaWithFormat = &Schema{
		Type:   Number,
		Format: Double,
	}

As well as:

	int64Schema = &Schema{
		Type: Integer,
	}
	int64SchemaWithFormat = &Schema{
		Type:   Integer,
		Format: Int64,
	}

We need to fix comparison to make such schemas equal.

Checklist:

  • Have a look at the Tigris doc once again to make sure that int64 and double are the only cases that can cause different schemas need to be equal.
  • Implement the changes and add tests to check that the comparison works equally for the different schemas that describe the same data type.
@rumyantseva rumyantseva added the code/enhancement Some user-visible feature could work better label Jul 18, 2022
@rumyantseva rumyantseva mentioned this issue Jul 18, 2022
10 tasks
@AlekSi AlekSi added not ready Issues that are not ready to be worked on; PRs that should skip CI code/tigris labels Jul 18, 2022
@rumyantseva rumyantseva changed the title [wip] tjson: Fix schema comparison tjson: Fix schema comparison Jul 19, 2022
@rumyantseva
Copy link
Member Author

@AlekSi I finalized the description for this task. Please have a look!

@AlekSi AlekSi added this to the v0.6.0 milestone Jul 20, 2022
@AlekSi AlekSi removed the not ready Issues that are not ready to be worked on; PRs that should skip CI label Jul 20, 2022
@rumyantseva rumyantseva added code/chore Code maintenance improvements and removed code/enhancement Some user-visible feature could work better labels Jul 26, 2022
AlekSi pushed a commit that referenced this issue Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code/chore Code maintenance improvements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants