davidradl commented on code in PR #79:
URL: 
https://github.com/apache/flink-connector-jdbc/pull/79#discussion_r1443983356


##########
flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/table/JdbcFilterPushdownPreparedStatementVisitor.java:
##########
@@ -46,8 +50,53 @@
 public class JdbcFilterPushdownPreparedStatementVisitor
         extends ExpressionDefaultVisitor<Optional<ParameterizedPredicate>> {
 
+    protected static final String PUSHDOWN_PREDICATE_PLACEHOLDER = "?";
+
+    protected static final String OPERATOR_EQUALS = "=";
+
+    protected static final String OPERATOR_LESS_THAN = "<";
+
+    protected static final String OPERATOR_LESS_THAN_OR_EQUAL = "<=";
+
+    protected static final String OPERATOR_GREATER_THAN = ">";
+
+    protected static final String OPERATOR_GREATER_THAN_OR_EQUAL = ">=";
+
+    protected static final String OPERATOR_NOT_EQUALS = "<>";
+
+    protected static final String OPERATOR_OR = "OR";
+
+    protected static final String OPERATOR_AND = "AND";
+
+    protected static final String OPERATOR_LIKE = "LIKE";
+
+    protected static final String OPERATOR_IS_NULL = "IS NULL";
+
+    protected static final String OPERATOR_IS_NOT_NULL = "IS NOT NULL";
+
+    private static String[] simpleBinaryOperatorValues =
+            new String[] {
+                OPERATOR_EQUALS,
+                OPERATOR_LESS_THAN,
+                OPERATOR_LESS_THAN_OR_EQUAL,
+                OPERATOR_GREATER_THAN,
+                OPERATOR_GREATER_THAN_OR_EQUAL,
+                OPERATOR_NOT_EQUALS,
+                OPERATOR_LIKE
+            };
+    private static String[] unaryOperatorValues =
+            new String[] {OPERATOR_IS_NULL, OPERATOR_IS_NOT_NULL};
+

Review Comment:
   Hi @snuyanzin , I thought as I was using these literals multiple times 
constants would be good practise. If there are existing constants I can use 
instead - that would be even better. For example,  I look at 
BuiltInFunctionDefinition.EQUALS in the debugger and do not see how I can get 
the call syntax from it.   I see that HiveTableUtils in the core Flink repo 
uses    FUNC_TO_STR to associate the call syntax with the build in function. I 
will change to using that style as there is precedent.  



-- 
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: issues-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to