amalsaidov opened a new pull request, #5414: URL: https://github.com/apache/calcite/pull/5414
## Jira Link [CALCITE-7890](https://issues.apache.org/jira/browse/CALCITE-7890) ## Changes Proposed `SqlDescriptorOperator.checkOperandTypes` (added in CALCITE-3382) never ran, and its condition was inverted: ```java if (!(operand instanceof SqlIdentifier) || ((SqlIdentifier) operand).isSimple()) { ``` It threw on a simple identifier, which is exactly what a valid `descriptor(rowtime)` is, accepted everything else, and returned `true` even for an invalid operand when `throwOnFailure` was false. `deriveType` returned `COLUMN_LIST` without ever calling it, so DESCRIPTOR itself never enforced that its arguments are column names. Only TUMBLE, HOP and SESSION rejected a bad descriptor, in `SqlWindowTableFunction.validateColumnNames` (CALCITE-7377). This PR: - Makes `deriveType` call `checkOperandTypes`, so every DESCRIPTOR is checked, not only the three window functions. - Rewrites `checkOperandTypes` to require each operand to be an identifier, fail with `descriptorMustBeIdentifier` ("The argument of DESCRIPTOR must be an identifier"), and return `false` on failure. The operator-level check stays at the identifier level rather than requiring a simple identifier, because by the time `deriveType` runs the validator has already qualified an unqualified name (`ename` becomes `EMP.ENAME`). The stricter rule, that each name is a simple identifier that resolves to a column, is still applied by `validateColumnNames`, which runs on the names as written. So `descriptor(1 + 2)` now fails in any function, while `descriptor(rowtime)` keeps validating and the existing window-function behavior is unchanged. A `DESCRIPTOR_CONSUMER` scalar function is added to `MockSqlOperatorTable` so `SqlValidatorTest#testDescriptor` can cover a DESCRIPTOR used outside a window table function: `descriptor(ename, deptno)` validates and `descriptor(1 + 2)` fails. The new case fails before this change and passes after it; the existing TUMBLE/HOP/SESSION tests are unaffected. Built and tested locally: `./gradlew :core:test --tests SqlValidatorTest.testDescriptor` and the TUMBLE/HOP/SESSION descriptor tests pass, and `checkstyleMain`/`checkstyleTest`/`forbiddenApisMain` are clean for `core` and `testkit`. AI-assisted; directed, reviewed and tested by the author (amalsaidov). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
