Repository navigation
Conversation
5fa2a02 to
12bfc5b
Compare
Reuse SqlNameMatcher for field-name ambiguity and JOIN-column uniqueness checks. Retain all exact-name candidates in RelRecordType so duplicate checks can reuse indexed lookup while insensitive matching still scans.
Add an independent lazy case-insensitive field-name index to RelRecordType. Retain duplicate candidates and preserve runtime Unicode matching semantics.
12bfc5b to
474da46
Compare
|
|
|
||
| /** Candidate indexes are derived from the fields and rebuilt on demand. */ | ||
| private transient volatile @Nullable Map<String, List<RelDataTypeField>> fieldNameMap; | ||
| private transient volatile @Nullable Map<CaseInsensitiveFieldName, List<RelDataTypeField>> |
There was a problem hiding this comment.
why does this have to be volatile?
| * @param elideRecord Whether to find fields nested within records | ||
| * @return Matching fallback field, or null if there is none | ||
| */ | ||
| protected @Nullable RelDataTypeField resolveFieldFallback(String fieldName, |
There was a problem hiding this comment.
does this method have to be protected?
| return null; | ||
| } | ||
|
|
||
| /** Returns a fallback after declared, nested, and extra-field lookup fail. |
There was a problem hiding this comment.
I don't understand: what is a fallback?
|
|
||
| /** Returns all declared fields matching a name, in field order. | ||
| * | ||
| * <p>Duplicate matches are retained. This method does not synthesize matches |
There was a problem hiding this comment.
what does it mean "does not synthesize matches"?
An empty list is returned when there are no matches? Then maybe "All matching fields" is enough.
| return fields.build(); | ||
| } | ||
|
|
||
| /** Counts occurrences of a field name among a row type's declared fields. |
There was a problem hiding this comment.
I usually think of frequency as a ratio, and a count as absolute.
| * @param fieldName Name to count | ||
| * @return Number of matching field names according to this matcher | ||
| */ | ||
| default int fieldFrequency(RelDataType rowType, String fieldName) { |
There was a problem hiding this comment.
why is this method needed?
| } | ||
|
|
||
| @Override public List<RelDataTypeField> fields(RelDataType rowType, String fieldName) { | ||
| if (rowType instanceof RelDataTypeImpl) { |
There was a problem hiding this comment.
this is ugly.
Can you think of making this nicer?
Maybe with some interface?
| } | ||
| } | ||
|
|
||
| private static List<RelDataTypeField> scan(List<RelDataTypeField> fields, |
There was a problem hiding this comment.
do you really need this many tests?
Jira Link
CALCITE-7918
Changes Proposed
The improvements are separated into two commits for review:
SqlNameMatcher, retaining duplicate fields in the exact-name index.RelRecordType, preserving duplicate matches and runtime Unicode matching semantics.Validation
27 focused tests passed: 11 field-lookup tests, 6 case-insensitive-index tests, 2 matcher-extension tests, and 8 SQL-validator tests. Core Checkstyle (main/test), Autostyle, Javadoc and
git diff --checkpassed.The source tree is identical to the validated version; this update only consolidates the commits into this PR and clarifies their subjects. The full Gradle build was not run.
Previously posted timings describe the original combined patch, including qualified-column reuse, which remains a separate follow-up.