Consistent hashing and equal methods - #1732
Open
CaiJimmy wants to merge 53 commits into
Open
Conversation
…sAndHashCode annotation across multiple classes
- Introduced AbstractNodeItem to centralize node equality and hashCode logic for XML items. - Updated AttributeItem, CommentItem, DocumentItem, ElementItem, NamespaceItem, ProcessingInstructionItem, TextItem to extend AbstractNodeItem, removing redundant equals and hashCode methods. - Created AbstractItemType to centralize equality handling for item types, allowing named types to compare by expanded name and structural types to use canonical keys. - Refactored various item types (ArrayItemType, AtomicItemType, etc.) to extend AbstractItemType, simplifying equality checks and hashCode implementations. - Updated MapItemType to improve equality handling and structural checks. - Adjusted ItemType interface to streamline equality checks across item types.
QT3 Test Results - Parser:
|
| Suite | Test file | Test case |
|---|---|---|
app |
app/UseCaseR.xml:rdb-queries-results-q12 |
|
fn |
fn/deep-equal.xml:cbcl-deep-equal-008 |
|
fn |
fn/deep-equal.xml:cbcl-deep-equal-010 |
|
fn |
fn/distinct-values.xml:K-SeqDistinctValuesFunc-2 |
|
fn |
fn/distinct-values.xml:K2-SeqDistinctValuesFunc-1 |
|
fn |
fn/distinct-values.xml:cbcl-distinct-values-016 |
|
fn |
fn/sort.xml:fn-sort-error-4 |
|
misc |
misc/CombinedErrorCodes.xml:FOCH0002_3 |
|
misc |
misc/HigherOrderFunctions.xml:hof-907 |
|
op |
op/same-key.xml:same-key-002 |
|
op |
op/same-key.xml:same-key-006 |
|
op |
op/same-key.xml:same-key-008 |
|
op |
op/same-key.xml:same-key-013 |
|
op |
op/same-key.xml:same-key-014 |
|
op |
op/same-key.xml:same-key-015 |
|
op |
op/same-key.xml:same-key-016 |
|
op |
op/same-key.xml:same-key-017 |
|
op |
op/same-key.xml:same-key-018 |
|
op |
op/same-key.xml:same-key-019 |
|
op |
op/same-key.xml:same-key-020 |
|
op |
op/same-key.xml:same-key-025 |
|
op |
op/same-key.xml:same-key-026 |
|
prod |
prod/AxisStep.ancestor-or-self.xml:K2-ancestor-or-selfAxis-1 |
|
prod |
prod/AxisStep.ancestor-or-self.xml:K2-ancestor-or-selfAxis-2 |
|
prod |
prod/AxisStep.following-sibling.xml:K2-following-siblingAxis-10 |
|
prod |
prod/AxisStep.xml:Axes107 |
|
prod |
prod/AxisStep.xml:Axes108 |
|
prod |
prod/AxisStep.xml:Axes109 |
|
prod |
prod/AxisStep.xml:Axes110 |
|
prod |
prod/AxisStep.xml:K2-Axes-48 |
|
prod |
prod/AxisStep.xml:K2-Axes-68 |
|
prod |
prod/AxisStep.xml:K2-Axes-70 |
|
prod |
prod/DirElemContent.namespace.xml:K2-DirectConElemNamespace-23 |
|
prod |
prod/EQName.xml:eqname-001 |
|
prod |
prod/EQName.xml:eqname-008 |
|
prod |
prod/EQName.xml:eqname-009 |
|
prod |
prod/EQName.xml:eqname-010 |
|
prod |
prod/EQName.xml:eqname-011 |
|
prod |
prod/InlineFunctionExpr.xml:inline-fn-029 |
|
prod |
prod/Predicate.xml:K2-Predicates-7 |
|
prod |
prod/StepExpr.xml:K2-Steps-14 |
|
prod |
prod/SwitchExpr.xml:switch-009 |
|
prod |
prod/SwitchExpr.xml:switch-010 |
|
prod |
prod/VarDecl.external.xml:extvardeclwithtype-23 |
|
prod |
prod/WhereClause.xml:cbcl-left-outer-join-004 |
Summary of passed tests for jsoniq
| Test Suite | Passing | Failing | Errors | Skipped | Total |
|---|---|---|---|---|---|
app |
1984 | 39 | 119 | 15 | 2157 |
array |
207 | 0 | 1 | 1 | 209 |
fn |
10020 | 380 | 210 | 525 | 11135 |
map |
220 | 0 | 0 | 0 | 220 |
math |
149 | 0 | 0 | 0 | 149 |
misc |
700 | 71 | 32 | 68 | 871 |
op |
4160 | 64 | 70 | 72 | 4366 |
prod |
9851 | 545 | 388 | 1423 | 12207 |
ser |
321 | 21 | 1 | 0 | 343 |
xs |
147 | 6 | 0 | 11 | 164 |
| Total | 27759 | 1126 | 821 | 2115 | 31821 |
Full analysis report: see analysis-jsoniq in artifacts.
QT3 Test Results - Parser:
|
| Suite | Test file | Test case |
|---|---|---|
app |
app/UseCaseR.xml:rdb-queries-results-q12 |
|
fn |
fn/deep-equal.xml:cbcl-deep-equal-008 |
|
fn |
fn/deep-equal.xml:cbcl-deep-equal-010 |
|
fn |
fn/distinct-values.xml:K-SeqDistinctValuesFunc-2 |
|
fn |
fn/distinct-values.xml:K2-SeqDistinctValuesFunc-1 |
|
fn |
fn/distinct-values.xml:cbcl-distinct-values-016 |
|
fn |
fn/sort.xml:fn-sort-error-4 |
|
misc |
misc/CombinedErrorCodes.xml:FOCH0002_3 |
|
misc |
misc/HigherOrderFunctions.xml:hof-907 |
|
op |
op/same-key.xml:same-key-002 |
|
op |
op/same-key.xml:same-key-006 |
|
op |
op/same-key.xml:same-key-008 |
|
op |
op/same-key.xml:same-key-013 |
|
op |
op/same-key.xml:same-key-014 |
|
op |
op/same-key.xml:same-key-015 |
|
op |
op/same-key.xml:same-key-016 |
|
op |
op/same-key.xml:same-key-017 |
|
op |
op/same-key.xml:same-key-018 |
|
op |
op/same-key.xml:same-key-019 |
|
op |
op/same-key.xml:same-key-020 |
|
op |
op/same-key.xml:same-key-025 |
|
op |
op/same-key.xml:same-key-026 |
|
prod |
prod/AxisStep.ancestor-or-self.xml:K2-ancestor-or-selfAxis-1 |
|
prod |
prod/AxisStep.ancestor-or-self.xml:K2-ancestor-or-selfAxis-2 |
|
prod |
prod/AxisStep.following-sibling.xml:K2-following-siblingAxis-10 |
|
prod |
prod/AxisStep.xml:Axes107 |
|
prod |
prod/AxisStep.xml:Axes108 |
|
prod |
prod/AxisStep.xml:Axes109 |
|
prod |
prod/AxisStep.xml:Axes110 |
|
prod |
prod/AxisStep.xml:K2-Axes-48 |
|
prod |
prod/AxisStep.xml:K2-Axes-68 |
|
prod |
prod/AxisStep.xml:K2-Axes-70 |
|
prod |
prod/CompNamespaceConstructor.xml:nscons-012 |
|
prod |
prod/DirElemContent.namespace.xml:K2-DirectConElemNamespace-23 |
|
prod |
prod/EQName.xml:eqname-001 |
|
prod |
prod/EQName.xml:eqname-008 |
|
prod |
prod/EQName.xml:eqname-009 |
|
prod |
prod/EQName.xml:eqname-010 |
|
prod |
prod/EQName.xml:eqname-011 |
|
prod |
prod/InlineFunctionExpr.xml:inline-fn-029 |
|
prod |
prod/OrderByClause.xml:orderbylocal-43 |
|
prod |
prod/OrderByClause.xml:orderbylocal-44 |
|
prod |
prod/OrderByClause.xml:orderbywithout-13 |
|
prod |
prod/OrderByClause.xml:orderbywithout-20 |
|
prod |
prod/OrderByClause.xml:orderbywithout-29 |
|
prod |
prod/OrderByClause.xml:orderbywithout-36 |
|
prod |
prod/OrderByClause.xml:orderbywithout-7 |
|
prod |
prod/Predicate.xml:K2-Predicates-7 |
|
prod |
prod/StepExpr.xml:K2-Steps-14 |
|
prod |
prod/SwitchExpr.xml:switch-009 |
Summary of passed tests for xquery
| Test Suite | Passing | Failing | Errors | Skipped | Total |
|---|---|---|---|---|---|
app |
2024 | 38 | 88 | 7 | 2157 |
array |
207 | 0 | 1 | 1 | 209 |
fn |
10074 | 351 | 212 | 498 | 11135 |
map |
220 | 0 | 0 | 0 | 220 |
math |
149 | 0 | 0 | 0 | 149 |
misc |
722 | 62 | 27 | 60 | 871 |
op |
4283 | 15 | 27 | 41 | 4366 |
prod |
10614 | 430 | 317 | 846 | 12207 |
ser |
343 | 0 | 0 | 0 | 343 |
xs |
153 | 0 | 0 | 11 | 164 |
| Total | 28789 | 896 | 672 | 1464 | 31821 |
Full analysis report: see analysis-xquery in artifacts.
…RDD execution, and grouping.
…Item, AbstractAtomicItem, and AbstractMapItem
…ses for boundary-space preservation
…do the change to DirectConstructorUtils
…oved boundary space handling and update test cases for consistency in whitespace preservation
CaiJimmy
changed the title
…hance AtomicDeepEqual to handle untyped atomic items
…tIterator to use AtomicDeepEqual; update IndexOfFunctionIterator for NaN handling; enhance DeepEqualFunctionIterator to throw exceptions for function comparisons; adjust AtomicValueComparison documentation; add new test cases for deep-equal functionality.
Consistent with arbitraryPrecisionDecimal and same-key
…llationSupport; update comparison logic to utilize active collation for distinct value checks.
… collation; add collation handling in equality and hash methods.
…on; update ItemFactory and CastIterator for new DecimalItem constructor; modify FloatItem and DoubleItem to use BigDecimal.valueOf for casting; adjust ValidateTypeIterator to return BigDecimal from string value.
CaiJimmy
commented
Jul 27, 2026
| * raise dynamic errors, and do not form an equivalence relation. The XDM {@code op:same-key} | ||
| * relation is stable and is already the equality required by maps. | ||
| */ | ||
| public final class AtomicItemEquivalence { |
Collaborator
Author
There was a problem hiding this comment.
Logic extracted from src/main/java/org/rumbledb/items/ItemSameKeyComparator.java
CaiJimmy
commented
Jul 27, 2026
| public MapWithRemovedEntryItem(Item original, List<Item> removedKeys) { | ||
| this.original = original; | ||
| this.removedKeys = new TreeSet<>(new ItemSameKeyComparator()); | ||
| this.removedKeys = new HashSet<>(); |
Collaborator
Author
There was a problem hiding this comment.
Same here, now we can use HashSet instead of TreeSet for better performance
CaiJimmy
commented
Jul 27, 2026
Comment on lines
+38
to
+46
| public final boolean equals(Object other) { | ||
| if (this == other) { | ||
| return true; | ||
| } | ||
| if (!(other instanceof AbstractItemType otherType)) { | ||
| return false; | ||
| } | ||
| return this.equalityKey().equals(otherType.equalityKey()); | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
If equalityKey is not overridden, it will compare by name (old behavior)
… of exact binary-derived value
CaiJimmy
marked this pull request as ready for review
July 27, 2026 18:10
…nal fields and Lombok's @Getter for improved immutability and cleaner code
…parisonKey for improved immutability
…ata and adjust DistinctValuesFunctionIterator accordingly
CaiJimmy
marked this pull request as draft
July 28, 2026 11:41
CaiJimmy
marked this pull request as ready for review
July 28, 2026 11:41
# Conflicts: # src/main/java/org/rumbledb/items/UntypedAtomicItem.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow up of #1731
Refine item comparison and hashing to make same-key semantics consistent across atomic, structured, and sequence values. Done by creating multiple Abstract classes for these types that defines
hashCodeandequalmethods in a centralized way.This fixes collation-aware string comparison, Gregorian temporal keys, numeric equality/hash behavior, and edge cases in arrays and objects. And many duplicated codes are removed.
Also preserves concise decimal lexical values when values cross DataFrame boundaries, avoiding floating-point artifacts in output, and removes an unsupported DataFrame implementation of distinct-values so it consistently uses XDM-correct RDD semantics.
With consistent hashing code implementation, we can now use HashMap and HashSet instead of TreeMap and TreeSet and get a better performance (O(1) lookup)