You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/05/30 05:14:28 UTC
[GitHub] [spark] AngersZhuuuu opened a new pull request, #36723: [SPARK39337][SQL] Refactor DescribeTableExec to remove duplicate filters.
AngersZhuuuu opened a new pull request, #36723:
URL: https://github.com/apache/spark/pull/36723
### What changes were proposed in this pull request?
Refactor DescribeTableExec to remove duplicate filters.
### Why are the changes needed?
Refactor code
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Not need
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu closed pull request #36723: [SPARK-39337][SQL] Refactor DescribeTableExec to remove duplicate filters
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu closed pull request #36723: [SPARK-39337][SQL] Refactor DescribeTableExec to remove duplicate filters
URL: https://github.com/apache/spark/pull/36723
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on pull request #36723: [SPARK-39337][SQL] Refactor DescribeTableExec to remove duplicate filters
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on PR #36723:
URL: https://github.com/apache/spark/pull/36723#issuecomment-1147096805
> Gentle ping, @AngersZhuuuu .
Can't find a efficient way to keep the order, will close this.
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #36723: [SPARK-39337][SQL] Refactor DescribeTableExec to remove duplicate filters
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #36723:
URL: https://github.com/apache/spark/pull/36723#discussion_r884759644
##########
sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala:
##########
@@ -147,10 +147,10 @@ class DataSourceV2SQLSuite
Array("", "", ""),
Array("# Detailed Table Information", "", ""),
Array("Name", "testcat.table_name", ""),
- Array("Comment", "this is a test table", ""),
- Array("Location", "file:/tmp/testcat/table_name", ""),
- Array("Provider", "foo", ""),
Array(TableCatalog.PROP_OWNER.capitalize, defaultUser, ""),
+ Array("Provider", "foo", ""),
+ Array("Location", "file:/tmp/testcat/table_name", ""),
+ Array("Comment", "this is a test table", ""),
Review Comment:
Are you saying that the order of output is different after your change?
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #36723: [SPARK-39337][SQL] Refactor DescribeTableExec to remove duplicate filters
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #36723:
URL: https://github.com/apache/spark/pull/36723#discussion_r884993022
##########
sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala:
##########
@@ -147,10 +147,10 @@ class DataSourceV2SQLSuite
Array("", "", ""),
Array("# Detailed Table Information", "", ""),
Array("Name", "testcat.table_name", ""),
- Array("Comment", "this is a test table", ""),
- Array("Location", "file:/tmp/testcat/table_name", ""),
- Array("Provider", "foo", ""),
Array(TableCatalog.PROP_OWNER.capitalize, defaultUser, ""),
+ Array("Provider", "foo", ""),
+ Array("Location", "file:/tmp/testcat/table_name", ""),
+ Array("Comment", "this is a test table", ""),
Review Comment:
@AngersZhuuuu . Yes, unfortunately, it matters. Personally, I'm negative if there is no way to avoid the side-effects.
> Hmmm, dose the order matter? We can add an sort like normal properties to make it stable.
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #36723: [SPARK-39337][SQL] Refactor DescribeTableExec to remove duplicate filters
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #36723:
URL: https://github.com/apache/spark/pull/36723#discussion_r884993022
##########
sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala:
##########
@@ -147,10 +147,10 @@ class DataSourceV2SQLSuite
Array("", "", ""),
Array("# Detailed Table Information", "", ""),
Array("Name", "testcat.table_name", ""),
- Array("Comment", "this is a test table", ""),
- Array("Location", "file:/tmp/testcat/table_name", ""),
- Array("Provider", "foo", ""),
Array(TableCatalog.PROP_OWNER.capitalize, defaultUser, ""),
+ Array("Provider", "foo", ""),
+ Array("Location", "file:/tmp/testcat/table_name", ""),
+ Array("Comment", "this is a test table", ""),
Review Comment:
@AngersZhuuuu . Yes, unfortunately, it matters. Personally, I'm -1 if there is no way to avoid the side-effects.
> Hmmm, dose the order matter? We can add an sort like normal properties to make it stable.
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on a diff in pull request #36723: [SPARK-39337][SQL] Refactor DescribeTableExec to remove duplicate filters
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on code in PR #36723:
URL: https://github.com/apache/spark/pull/36723#discussion_r884694923
##########
sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala:
##########
@@ -147,10 +147,10 @@ class DataSourceV2SQLSuite
Array("", "", ""),
Array("# Detailed Table Information", "", ""),
Array("Name", "testcat.table_name", ""),
- Array("Comment", "this is a test table", ""),
- Array("Location", "file:/tmp/testcat/table_name", ""),
- Array("Provider", "foo", ""),
Array(TableCatalog.PROP_OWNER.capitalize, defaultUser, ""),
+ Array("Provider", "foo", ""),
+ Array("Location", "file:/tmp/testcat/table_name", ""),
+ Array("Comment", "this is a test table", ""),
Review Comment:
> It would be great if we don't change this order in this **refactoring** if possible. Do you think we can avoid this, @AngersZhuuuu ?
Code will be complex, how about use `sort` to make it have a stable order?
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #36723: [SPARK-39337][SQL] Refactor DescribeTableExec to remove duplicate filters
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #36723:
URL: https://github.com/apache/spark/pull/36723#discussion_r884581701
##########
sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala:
##########
@@ -147,10 +147,10 @@ class DataSourceV2SQLSuite
Array("", "", ""),
Array("# Detailed Table Information", "", ""),
Array("Name", "testcat.table_name", ""),
- Array("Comment", "this is a test table", ""),
- Array("Location", "file:/tmp/testcat/table_name", ""),
- Array("Provider", "foo", ""),
Array(TableCatalog.PROP_OWNER.capitalize, defaultUser, ""),
+ Array("Provider", "foo", ""),
+ Array("Location", "file:/tmp/testcat/table_name", ""),
+ Array("Comment", "this is a test table", ""),
Review Comment:
It would be great if we don't change this order in this **refactoring** if possible.
Do you think we can avoid this, @AngersZhuuuu ?
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on a diff in pull request #36723: [SPARK-39337][SQL] Refactor DescribeTableExec to remove duplicate filters
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on code in PR #36723:
URL: https://github.com/apache/spark/pull/36723#discussion_r884771145
##########
sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala:
##########
@@ -147,10 +147,10 @@ class DataSourceV2SQLSuite
Array("", "", ""),
Array("# Detailed Table Information", "", ""),
Array("Name", "testcat.table_name", ""),
- Array("Comment", "this is a test table", ""),
- Array("Location", "file:/tmp/testcat/table_name", ""),
- Array("Provider", "foo", ""),
Array(TableCatalog.PROP_OWNER.capitalize, defaultUser, ""),
+ Array("Provider", "foo", ""),
+ Array("Location", "file:/tmp/testcat/table_name", ""),
+ Array("Comment", "this is a test table", ""),
Review Comment:
>
Hmmm, dose the order matter? We can add an sort like normal properties to make it stable.
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #36723: [SPARK-39337][SQL] Refactor DescribeTableExec to remove duplicate filters
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #36723:
URL: https://github.com/apache/spark/pull/36723#issuecomment-1146909417
Gentle ping, @AngersZhuuuu .
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on pull request #36723: [SPARK39337][SQL] Refactor DescribeTableExec to remove duplicate filters.
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on PR #36723:
URL: https://github.com/apache/spark/pull/36723#issuecomment-1140728763
> Could you revise the PR description by elaborating a little more, @AngersZhuuuu ?
How about current
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on pull request #36723: [SPARK39337][SQL] Refactor DescribeTableExec to remove duplicate filters.
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on PR #36723:
URL: https://github.com/apache/spark/pull/36723#issuecomment-1140706671
Gentle ping @HyukjinKwon @dongjoon-hyun
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org