kbendick commented on a change in pull request #3460:
URL: https://github.com/apache/iceberg/pull/3460#discussion_r742232207
##########
File path:
spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/Spark3Util.java
##########
@@ -636,17 +635,17 @@ public String or(String leftResult, String rightResult) {
case NOT_NAN:
return "not_nan(" + pred.ref().name() + ")";
case LT:
- return pred.ref().name() + " < " + sqlString(pred.literal());
+ return pred.ref().name() + " < " + pred.literal();
case LT_EQ:
- return pred.ref().name() + " <= " + sqlString(pred.literal());
+ return pred.ref().name() + " <= " + pred.literal();
case GT:
- return pred.ref().name() + " > " + sqlString(pred.literal());
+ return pred.ref().name() + " > " + pred.literal();
case GT_EQ:
- return pred.ref().name() + " >= " + sqlString(pred.literal());
+ return pred.ref().name() + " >= " + pred.literal();
case EQ:
- return pred.ref().name() + " = " + sqlString(pred.literal());
+ return pred.ref().name() + " = " + pred.literal();
case NOT_EQ:
- return pred.ref().name() + " != " + sqlString(pred.literal());
+ return pred.ref().name() + " != " + pred.literal();
Review comment:
Why did we get rid of the `sqlString` function here? Is there a change
that removes the need to quote strings for example?
Also, since the representation of `ByteBuffer` is potentially engine
specific, would it be better to add the conversion to the `sqlString` here for
Spark (e.g. using the leading capital X, like `X'123456'`)?
This would allow other engines to handle it themselves (e.g. if Flink
doesn't use leading `X` format).
--
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]
