gianm commented on code in PR #19711:
URL: https://github.com/apache/druid/pull/19711#discussion_r3636114219
##########
sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidSqlValidator.java:
##########
@@ -536,20 +540,25 @@ private RelDataType validateTargetType(
// disable sealed mode validation if catalog validation is disabled.
final boolean isStrict = tableMetadata.isSealed();
+ // Base table layouts (which require 'sealed') may declare ingest-time
virtual columns; the columns those virtual
+ // columns consume are allowed in the source even though they are not
stored (e.g. a raw column feeding an
+ // expression that computes a stored clustering column).
+ final Set<String> baseTableVirtualColumnInputs =
getBaseTableVirtualColumnInputs(tableMetadata.baseTableMetadata());
Review Comment:
I think the automated review is pointing out a problem with this:
- columns `[__time, tenant_lower]` + virtual column `tenant_lower =
lower("tenant")`
- user runs `INSERT … SELECT __time, 'tenant1' AS tenant_lower FROM …`
The validator would accept the SQL, but the virtual column `tenant_lower`
would be applied and would yield `null`. So the value `tenant1` wouldn't get
stored.
IMO, in this case the best thing to do is fail validation. Probably the
validator should reject columns with the same name as a virtual column, because
they'd end up being ignored if accepted.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
