Re: [PR] [SPARK-47044] Add executed query for JDBC external datasources to explain output [spark]

2024-02-19 Thread via GitHub


urosstan-db commented on PR #45102:
URL: https://github.com/apache/spark/pull/45102#issuecomment-1952678599

   > is there any security concern? do we need to do redaction?
   
   There should not be any concern if we don't save query result anywhere?
   Spark UI will also create UI for plan on using same methods as Explain 
Formatted, is it saved somewhere or accessible to anyone else except the 
customer?


-- 
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-47044] Add executed query for JDBC external datasources to explain output [spark]

2024-02-19 Thread via GitHub


urosstan-db commented on code in PR #45102:
URL: https://github.com/apache/spark/pull/45102#discussion_r1494692948


##
sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcSQLQueryBuilder.scala:
##
@@ -81,17 +81,21 @@ class JdbcSQLQueryBuilder(dialect: JdbcDialect, options: 
JDBCOptions) {
   /**
* Constructs the WHERE clause that following dialect's SQL syntax.
*/
-  def withPredicates(predicates: Array[Predicate], part: JDBCPartition): 
JdbcSQLQueryBuilder = {
+  def withPredicates(
+  predicates: Array[Predicate],
+  part: Option[JDBCPartition]): JdbcSQLQueryBuilder = {

Review Comment:
   We can do that, we can pass JDBCPartition(whereClause = null, idx = 0).
   Or maybe, we can skip withPredicates method from generateJDBCQuery (maybe a 
little bit nicer.



-- 
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-47044] Add executed query for JDBC external datasources to explain output [spark]

2024-02-19 Thread via GitHub


cloud-fan commented on PR #45102:
URL: https://github.com/apache/spark/pull/45102#issuecomment-1952643638

   is there any security concern? do we need to do redaction?


-- 
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-47044] Add executed query for JDBC external datasources to explain output [spark]

2024-02-19 Thread via GitHub


cloud-fan commented on code in PR #45102:
URL: https://github.com/apache/spark/pull/45102#discussion_r1494684198


##
sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcSQLQueryBuilder.scala:
##
@@ -81,17 +81,21 @@ class JdbcSQLQueryBuilder(dialect: JdbcDialect, options: 
JDBCOptions) {
   /**
* Constructs the WHERE clause that following dialect's SQL syntax.
*/
-  def withPredicates(predicates: Array[Predicate], part: JDBCPartition): 
JdbcSQLQueryBuilder = {
+  def withPredicates(
+  predicates: Array[Predicate],
+  part: Option[JDBCPartition]): JdbcSQLQueryBuilder = {

Review Comment:
   This is a public API and we can't change its signature. How about we pass a 
fake `JDBCPartition` in EXPLAIN?



-- 
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-47044] Add executed query for JDBC external datasources to explain output [spark]

2024-02-19 Thread via GitHub


urosstan-db commented on code in PR #45102:
URL: https://github.com/apache/spark/pull/45102#discussion_r1494338113


##
sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcSQLQueryBuilder.scala:
##
@@ -81,17 +81,20 @@ class JdbcSQLQueryBuilder(dialect: JdbcDialect, options: 
JDBCOptions) {
   /**
* Constructs the WHERE clause that following dialect's SQL syntax.
*/
-  def withPredicates(predicates: Array[Predicate], part: JDBCPartition): 
JdbcSQLQueryBuilder = {
+  def withPredicates(predicates: Array[Predicate],

Review Comment:
   Done



-- 
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-47044] Add executed query for JDBC external datasources to explain output [spark]

2024-02-14 Thread via GitHub


dtenedor commented on code in PR #45102:
URL: https://github.com/apache/spark/pull/45102#discussion_r1490403653


##
sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcSQLQueryBuilder.scala:
##
@@ -81,17 +81,20 @@ class JdbcSQLQueryBuilder(dialect: JdbcDialect, options: 
JDBCOptions) {
   /**
* Constructs the WHERE clause that following dialect's SQL syntax.
*/
-  def withPredicates(predicates: Array[Predicate], part: JDBCPartition): 
JdbcSQLQueryBuilder = {
+  def withPredicates(predicates: Array[Predicate],

Review Comment:
   please start these arguments on the next line, indented by +4 spaces each.



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