What is the problem the feature request solves?
CometLiteral.getSupportLevel checks the literal's type through supportedDataType with the default allowAnyStringType = true, so its StringType case accepts every collation (QueryPlanSerde.scala:615, 624-625), and convert writes the value with setStringVal (literals.scala:98-99). So a Literal of type StringType(UTF8_LCASE) is accepted, and its collation is not carried into the native plan. Nothing records that it was dropped.
A cast of a literal is one way to get such a literal. CometCast.getSupportLevel returns Compatible() for a cast whose child is a Literal, unless a variant type is involved, without calling isSupported (CometCast.scala:90-99), so the collation guard proposed in #5302 would not see it either. convert then folds the cast with cast.eval() into a new literal of the target type (:111-112). CAST('abc' AS STRING COLLATE UTF8_LCASE) therefore turns into a collated literal that CometLiteral accepts. ConstantFolding normally removes a cast like that before Comet sees it, but CometSqlFileTestSuite excludes ConstantFolding for every fixture (CometSqlFileTestSuite.scala:92), so Comet's own harness can reach this path.
The answer is right today, since folding produces the same bytes Spark would. The problem is the one #4489 described for CometCast: the behavior is implicit and untested, so the next change to either serde can alter it without anyone noticing. I have not built a query that returns a wrong result through this path.
Describe the potential solution
Two options, and I don't have a strong preference:
CometLiteral.getSupportLevel returns Unsupported when the literal's type has a non-default collation. Passing allowAnyStringType = false to supportedDataType would do it for a plain string literal, the way CometLocalTableScanExec already does (CometLocalTableScanExec.scala:146). CometLiteral also mixes in CometTypeShim, so hasNonDefaultStringCollation is in scope for nested types. The cost is that these literals, and the expressions around them, fall back to Spark.
- Keep accepting them, since the bytes match, and say so in a comment in
CometLiteral.
Either way, a test under CometSqlFileTestSuite with CAST('abc' AS STRING COLLATE UTF8_LCASE) would pin the choice, since that harness is the one that can reach this path.
Additional context
Found while working on #5302 (#4489). Discussion: #5302 (review)
Line numbers are from main at 646ff181.
What is the problem the feature request solves?
CometLiteral.getSupportLevelchecks the literal's type throughsupportedDataTypewith the defaultallowAnyStringType = true, so itsStringTypecase accepts every collation (QueryPlanSerde.scala:615, 624-625), andconvertwrites the value withsetStringVal(literals.scala:98-99). So aLiteralof typeStringType(UTF8_LCASE)is accepted, and its collation is not carried into the native plan. Nothing records that it was dropped.A cast of a literal is one way to get such a literal.
CometCast.getSupportLevelreturnsCompatible()for a cast whose child is aLiteral, unless a variant type is involved, without callingisSupported(CometCast.scala:90-99), so the collation guard proposed in #5302 would not see it either.convertthen folds the cast withcast.eval()into a new literal of the target type (:111-112).CAST('abc' AS STRING COLLATE UTF8_LCASE)therefore turns into a collated literal thatCometLiteralaccepts.ConstantFoldingnormally removes a cast like that before Comet sees it, butCometSqlFileTestSuiteexcludesConstantFoldingfor every fixture (CometSqlFileTestSuite.scala:92), so Comet's own harness can reach this path.The answer is right today, since folding produces the same bytes Spark would. The problem is the one #4489 described for
CometCast: the behavior is implicit and untested, so the next change to either serde can alter it without anyone noticing. I have not built a query that returns a wrong result through this path.Describe the potential solution
Two options, and I don't have a strong preference:
CometLiteral.getSupportLevelreturnsUnsupportedwhen the literal's type has a non-default collation. PassingallowAnyStringType = falsetosupportedDataTypewould do it for a plain string literal, the wayCometLocalTableScanExecalready does (CometLocalTableScanExec.scala:146).CometLiteralalso mixes inCometTypeShim, sohasNonDefaultStringCollationis in scope for nested types. The cost is that these literals, and the expressions around them, fall back to Spark.CometLiteral.Either way, a test under
CometSqlFileTestSuitewithCAST('abc' AS STRING COLLATE UTF8_LCASE)would pin the choice, since that harness is the one that can reach this path.Additional context
Found while working on #5302 (#4489). Discussion: #5302 (review)
Line numbers are from
mainat646ff181.