Re: [PR] [SPARK-48915][SQL][TESTS] Add some uncovered predicates(!=, <=, >, >=) in test cases of `GeneratedSubquerySuite` [spark]
wayneguow commented on PR #47386: URL: https://github.com/apache/spark/pull/47386#issuecomment-2233438709 cc @cloud-fan @andylam-db -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48915][SQL][TESTS] Add some uncovered predicates(!=, <=, >, >=) in test cases of `GeneratedSubquerySuite` [spark]
wayneguow commented on PR #47386: URL: https://github.com/apache/spark/pull/47386#issuecomment-2233445086 cc @cloud-fan @andylam-db -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48915][SQL][TESTS] Add some uncovered predicates(!=, <=, >, >=) in test cases of `GeneratedSubquerySuite` [spark]
andylam-db commented on code in PR #47386:
URL: https://github.com/apache/spark/pull/47386#discussion_r1681403062
##
sql/core/src/test/scala/org/apache/spark/sql/QueryGeneratorHelper.scala:
##
@@ -96,7 +109,10 @@ trait QueryGeneratorHelper {
// Subquery to be treated as a Relation.
val RELATION = Value
// Subquery is a Predicate - types of predicate subqueries.
-val SCALAR_PREDICATE_EQUALS, SCALAR_PREDICATE_LESS_THAN, IN, NOT_IN,
EXISTS, NOT_EXISTS = Value
+val SCALAR_PREDICATE_EQUALS, SCALAR_PREDICATE_NOT_EQUALS,
+SCALAR_PREDICATE_LESS_THAN, SCALAR_PREDICATE_LESS_THAN_OR_EQUALS,
+SCALAR_PREDICATE_GREATER_THAN, SCALAR_PREDICATE_GREATER_THAN_OR_EQUALS,
+IN, NOT_IN, EXISTS, NOT_EXISTS = Value
Review Comment:
NIT: Indentation
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48915][SQL][TESTS] Add some uncovered predicates(!=, <=, >, >=) in test cases of `GeneratedSubquerySuite` [spark]
wayneguow commented on PR #47386: URL: https://github.com/apache/spark/pull/47386#issuecomment-2233816574 > Thanks for the PR! > > Your implementation improves coverage of predicate types of scalar subquery in the WHERE clause, but I think the ticket is asking for different predicate types in the correlation predicate (within the subquery itself). Your PR is helpful of course, and I think we should merge it, but I think the ask on the ticket is different. @andylam-db Thank you for your explanation. In addition to the current PR, I may be able to open a new PR. -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48915][SQL][TESTS] Add some uncovered predicates(!=, <=, >, >=) in test cases of `GeneratedSubquerySuite` [spark]
wayneguow commented on code in PR #47386:
URL: https://github.com/apache/spark/pull/47386#discussion_r1681458803
##
connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/querytest/GeneratedSubquerySuite.scala:
##
@@ -182,8 +182,11 @@ class GeneratedSubquerySuite extends
DockerJDBCIntegrationSuite with QueryGenera
} else {
Review Comment:
I should add some other predicates like `NotEquals`, `LessThan`... (reuse
all newly added ones) here, is this right? @andylam-db
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48915][SQL][TESTS] Add some uncovered predicates(!=, <=, >, >=) in test cases of `GeneratedSubquerySuite` [spark]
wayneguow commented on code in PR #47386:
URL: https://github.com/apache/spark/pull/47386#discussion_r1681458803
##
connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/querytest/GeneratedSubquerySuite.scala:
##
@@ -182,8 +182,11 @@ class GeneratedSubquerySuite extends
DockerJDBCIntegrationSuite with QueryGenera
} else {
Review Comment:
If want to further solve the ticket, currently there is only `=`(`Equals`)
here, we should be able to generate Query for !=, <, <=, >, >= where the upper
layer calls. Is this right? @andylam-db
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48915][SQL][TESTS] Add some uncovered predicates(!=, <=, >, >=) in test cases of `GeneratedSubquerySuite` [spark]
andylam-db commented on code in PR #47386:
URL: https://github.com/apache/spark/pull/47386#discussion_r1681518062
##
connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/querytest/GeneratedSubquerySuite.scala:
##
@@ -182,8 +182,11 @@ class GeneratedSubquerySuite extends
DockerJDBCIntegrationSuite with QueryGenera
} else {
Review Comment:
Not sure I understand what you mean by `where the upper layer calls`.
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48915][SQL][TESTS] Add some uncovered predicates(!=, <=, >, >=) in test cases of `GeneratedSubquerySuite` [spark]
wayneguow commented on code in PR #47386:
URL: https://github.com/apache/spark/pull/47386#discussion_r1681545381
##
connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/querytest/GeneratedSubquerySuite.scala:
##
@@ -182,8 +182,11 @@ class GeneratedSubquerySuite extends
DockerJDBCIntegrationSuite with QueryGenera
} else {
Review Comment:
Oh, sorry, I didn't make it clear, I meant in line359-362,
`for {
subqueryOperator <- subqueryOperators
isDistinct <- distinctChoices(subqueryOperator)
}`
add another axis about `correlationConditions` here to cover all the
different predicates, not only hardcoded about `=` in `generateQuery` method.
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48915][SQL][TESTS] Add some uncovered predicates(!=, <=, >, >=) in test cases of `GeneratedSubquerySuite` [spark]
andylam-db commented on code in PR #47386:
URL: https://github.com/apache/spark/pull/47386#discussion_r1681547836
##
connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/querytest/GeneratedSubquerySuite.scala:
##
@@ -182,8 +182,11 @@ class GeneratedSubquerySuite extends
DockerJDBCIntegrationSuite with QueryGenera
} else {
Review Comment:
Sounds great!
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48915][SQL][TESTS] Add some uncovered predicates(!=, <=, >, >=) in test cases of `GeneratedSubquerySuite` [spark]
cloud-fan commented on PR #47386: URL: https://github.com/apache/spark/pull/47386#issuecomment-2235161996 thanks, merging to master! -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-48915][SQL][TESTS] Add some uncovered predicates(!=, <=, >, >=) in test cases of `GeneratedSubquerySuite` [spark]
cloud-fan closed pull request #47386: [SPARK-48915][SQL][TESTS] Add some uncovered predicates(!=, <=, >, >=) in test cases of `GeneratedSubquerySuite` URL: https://github.com/apache/spark/pull/47386 -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
