Vladimir Sitnikov created CALCITE-7890:
------------------------------------------
Summary: SqlDescriptorOperator.checkOperandTypes rejects simple
identifiers and is never called
Key: CALCITE-7890
URL: https://issues.apache.org/jira/browse/CALCITE-7890
Project: Calcite
Issue Type: Bug
Components: core
Affects Versions: 1.42.0
Reporter: Vladimir Sitnikov
{{SqlDescriptorOperator.checkOperandTypes}}, added in CALCITE-3382, throws when
an operand *is* a simple identifier:
{code:java}
if (!(operand instanceof SqlIdentifier)
|| ((SqlIdentifier) operand).isSimple()) {
if (throwOnFailure) {
throw SqlUtil.newContextException(operand.getParserPosition(),
RESOURCE.aliasMustBeSimpleIdentifier());
}
}
{code}
If the validator called it, every valid {{descriptor(rowtime)}} would fail with
"Table or column alias must be a simple identifier". The validator never calls
it: {{SqlDescriptorOperator.deriveType}} returns {{COLUMN_LIST}} without
calling {{validateOperands}}, and no other validator path calls
{{checkOperandTypes}} on a DESCRIPTOR. This is why
{{SqlValidatorTest#testDescriptor}} accepts {{descriptor(rowtime)}}. When
{{throwOnFailure}} is false, the method returns {{true}} for an invalid operand
as well.
As a result, the operator itself does not enforce that a DESCRIPTOR argument is
a list of column names (SQL:2016 8.15, {{<descriptor column list>}}). Only
TUMBLE, HOP, and SESSION check their DESCRIPTOR operands, in
{{SqlWindowTableFunction.validateColumnNames}}, which CALCITE-7377 added.
Seen on main at ce51f0e527 (1.43.0-SNAPSHOT), and in 1.42.0.
*Expected*, one of:
* {{SqlDescriptorOperator}} checks its operands, so that {{descriptor(1 + 2)}}
and {{descriptor(t.a)}} fail with "The argument of DESCRIPTOR must be an
identifier" (the {{descriptorMustBeIdentifier}} message from CALCITE-7377) in
any function, while {{descriptor(rowtime)}} still validates.
{{validateColumnNames}} then only needs to resolve the names. The DESCRIPTOR
entry in {{site/_docs/reference.md}} says "The interpretation of names is left
to the function"; it would need a sentence that each name must be a simple
identifier.
* Or, if the window functions are meant to own the check, the dead
{{checkOperandTypes}} override is removed.
CALCITE-3964 proposes an optional data type after each column name, so an
operand check here would have to allow for that if it lands.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
