HyukjinKwon commented on a change in pull request #35494:
URL: https://github.com/apache/spark/pull/35494#discussion_r806386913



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala
##########
@@ -194,6 +194,66 @@ abstract class JdbcDialect extends Serializable with 
Logging{
     case _ => value
   }
 
+  protected final val BINARY_COMPARISON = Set("=", "!=", "<=>", "<", "<=", 
">", ">=",
+    "+", "-", "*", "/", "%", "&&", "||", "AND", "OR", "&", "|", "^")
+
+  /**
+   * Converts V2 expression to String representing a SQL expression.
+   * @param expr The V2 expression to be converted.
+   * @return Converted value.
+   */
+  @Since("3.3.0")
+  def compileExpression(expr: Expression): Option[String] = expr match {
+    case l @ LiteralValue(_, _) => Some(l.toString)

Review comment:
       Yeah. There was a try to repalce them all but it was rejected as it's 
too invasive. We should better avoid them anyway.

##########
File path: 
sql/core/src/main/scala/org/apache/spark/sql/catalyst/util/ExpressionSQLBuilder.scala
##########
@@ -67,3 +122,8 @@ class ExpressionSQLBuilder(e: Expression) {
     case _ => None
   }
 }
+
+object GeneralExpression {
+  def apply(expressions: Array[V2Expression], name: String = "UNDEFINED"): 
GeneralSQLExpression =
+    new GeneralSQLExpression(name, expressions)

Review comment:
       I meant that removing this `object`, and dealing with this default 
argument in the constructor of `GeneralExpression` at the Java file.

##########
File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/connector/expressions/GeneralSQLExpression.java
##########
@@ -22,20 +22,34 @@
 import org.apache.spark.annotation.Evolving;
 
 /**
- * The general SQL string corresponding to expression.
+ * The general V2 expression corresponding to V1 expression.

Review comment:
       I meant documenting that this API should be used in the source that can 
understand SQL in this Javadoc.

##########
File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/connector/expressions/GeneralSQLExpression.java
##########
@@ -22,20 +22,34 @@
 import org.apache.spark.annotation.Evolving;
 
 /**
- * The general SQL string corresponding to expression.
+ * The general V2 expression corresponding to V1 expression.

Review comment:
       I meant documenting (mentioning) that this API should be used in the 
source that can understand SQL in this Javadoc.

##########
File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/connector/expressions/GeneralSQLExpression.java
##########
@@ -22,20 +22,34 @@
 import org.apache.spark.annotation.Evolving;
 
 /**
- * The general SQL string corresponding to expression.
+ * The general V2 expression corresponding to V1 expression.
  *
  * @since 3.3.0
  */
 @Evolving
 public class GeneralSQLExpression implements Expression, Serializable {

Review comment:
       Do you meant that we will use this for all V2 expressions? I thought 
this is one of the optional APIs.

##########
File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/connector/expressions/GeneralSQLExpression.java
##########
@@ -22,20 +22,34 @@
 import org.apache.spark.annotation.Evolving;
 
 /**
- * The general SQL string corresponding to expression.
+ * The general V2 expression corresponding to V1 expression.
  *
  * @since 3.3.0
  */
 @Evolving
 public class GeneralSQLExpression implements Expression, Serializable {

Review comment:
       Do you mean that we will use this for all V2 expressions? I thought this 
is one of the optional APIs.




-- 
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]

Reply via email to