You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "beliefer (via GitHub)" <gi...@apache.org> on 2023/10/13 02:59:41 UTC
[PR] [WIP][SPARK-45531][SQL][DOCS] Add more comments and rename some variable name for InjectRuntimeFilter [spark]
beliefer opened a new pull request, #43359:
URL: https://github.com/apache/spark/pull/43359
### What changes were proposed in this pull request?
### Why are the changes needed?
### Does this PR introduce _any_ user-facing change?
'No'.
### How was this patch tested?
N/A
### Was this patch authored or co-authored using generative AI tooling?
'No'.
--
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
Re: [PR] [SPARK-45531][SQL][DOCS] Add more comments and rename some variable name for InjectRuntimeFilter [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on PR #43359:
URL: https://github.com/apache/spark/pull/43359#issuecomment-1762593379
> what variables did you rename?
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-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
Re: [PR] [WIP][SPARK-45531][SQL][DOCS] Add more comments and rename some variable name for InjectRuntimeFilter [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #43359:
URL: https://github.com/apache/spark/pull/43359#discussion_r1357704746
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/InjectRuntimeFilter.scala:
##########
@@ -29,7 +29,9 @@ import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.sql.types._
/**
- * Insert a filter on one side of the join if the other side has a selective predicate.
+ * Insert a runtime filter on one side of the join (we call this side the application side) if
+ * we can extract a runtime filter from the other side (creation side). A simple case is that
+ * the creation side is a table scan with a selective filter.
* The filter could be an IN subquery (converted to a semi join), a bloom filter, or something
Review Comment:
```suggestion
* The runtime filter could be an IN subquery (converted to a semi join), a bloom filter, or something
```
--
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
Re: [PR] [WIP][SPARK-45531][SQL][DOCS] Add more comments and rename some variable name for InjectRuntimeFilter [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on PR #43359:
URL: https://github.com/apache/spark/pull/43359#issuecomment-1761418972
The failure is unrelated.
--
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
Re: [PR] [SPARK-45531][SQL][DOCS] Add more comments and rename some variable name for InjectRuntimeFilter [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on PR #43359:
URL: https://github.com/apache/spark/pull/43359#issuecomment-1763938740
@cloud-fan Thank you!
--
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
Re: [PR] [WIP][SPARK-45531][SQL][DOCS] Add more comments and rename some variable name for InjectRuntimeFilter [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #43359:
URL: https://github.com/apache/spark/pull/43359#issuecomment-1761466792
what variables did you rename?
--
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
Re: [PR] [SPARK-45531][SQL][DOCS] Add more comments and rename some variable name for InjectRuntimeFilter [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #43359:
URL: https://github.com/apache/spark/pull/43359#discussion_r1360067208
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/InjectRuntimeFilter.scala:
##########
@@ -29,48 +29,50 @@ import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.sql.types._
/**
- * Insert a filter on one side of the join if the other side has a selective predicate.
- * The filter could be an IN subquery (converted to a semi join), a bloom filter, or something
- * else in the future.
+ * Insert a runtime filter on one side of the join (we call this side the application side) if
+ * we can extract a runtime filter from the other side (creation side). A simple case is that
+ * the creation side is a table scan with a selective filter.
+ * The runtime filter could be an IN subquery (converted to a semi join), a bloom filter, or
+ * something else in the future.
Review Comment:
```
The runtime filter is logically an IN subquery with the join keys (converted to a semi join),
but can be something different physically, such as a bloom filter.
```
--
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
Re: [PR] [SPARK-45531][SQL][DOCS] Add more comments and rename some variable name for InjectRuntimeFilter [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan closed pull request #43359: [SPARK-45531][SQL][DOCS] Add more comments and rename some variable name for InjectRuntimeFilter
URL: https://github.com/apache/spark/pull/43359
--
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
Re: [PR] [SPARK-45531][SQL][DOCS] Add more comments and rename some variable name for InjectRuntimeFilter [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #43359:
URL: https://github.com/apache/spark/pull/43359#issuecomment-1763902168
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-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