You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "wankunde (via GitHub)" <gi...@apache.org> on 2024/09/04 10:06:34 UTC
[PR] [SPARK-44239][SQL][MINOR] Update default hugeVectorThreshold from 0 to -1 in code [spark]
wankunde opened a new pull request, #47988:
URL: https://github.com/apache/spark/pull/47988
### What changes were proposed in this pull request?
Minor fix: Update default hugeVectorThreshold from 0 to -1 in code to match the documentation.
Keep consistent with the documentation
### Why are the changes needed?
Keep code and documentation consistent
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Exists UT
### 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-44239][SQL] Update default hugeVectorThreshold from 0 to -1 in code [spark]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #47988:
URL: https://github.com/apache/spark/pull/47988#issuecomment-2329232164
Also cc @cloud-fan who reviewed and merged #41782 / SPARK-44239
--
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-44239][SQL][FOLLOWUP] Do not disable vector memory optimization when hugeVectorThreshold=0 to align its document [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #47988:
URL: https://github.com/apache/spark/pull/47988#discussion_r1744748355
##########
sql/core/src/main/java/org/apache/spark/sql/execution/vectorized/WritableColumnVector.java:
##########
@@ -72,7 +72,7 @@ public void reset() {
numNulls = 0;
}
- if (hugeVectorThreshold > 0 && capacity > hugeVectorThreshold) {
Review Comment:
`= 0` means we treat all vectors as huge vectors?
--
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-44239][SQL][FOLLOWUP] Do not disable vector memory optimization when hugeVectorThreshold=0 to align its document [spark]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn closed pull request #47988: [SPARK-44239][SQL][FOLLOWUP] Do not disable vector memory optimization when hugeVectorThreshold=0 to align its document
URL: https://github.com/apache/spark/pull/47988
--
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-44239][SQL][FOLLOWUP] Do not disable vector memory optimization when hugeVectorThreshold=0 to align its document [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #47988:
URL: https://github.com/apache/spark/pull/47988#discussion_r1746479952
##########
sql/core/src/main/java/org/apache/spark/sql/execution/vectorized/WritableColumnVector.java:
##########
@@ -72,7 +72,7 @@ public void reset() {
numNulls = 0;
}
- if (hugeVectorThreshold > 0 && capacity > hugeVectorThreshold) {
Review Comment:
ah I see
--
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-44239][SQL][FOLLOWUP] Do not disable vector memory optimization when hugeVectorThreshold=0 to align its document [spark]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #47988:
URL: https://github.com/apache/spark/pull/47988#issuecomment-2333375425
Merged to master. Thank you @wankunde and all
--
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-44239][SQL][FOLLOWUP] Do not disable vector memory optimization when hugeVectorThreshold=0 to align its document [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #47988:
URL: https://github.com/apache/spark/pull/47988#discussion_r1745565782
##########
sql/core/src/main/java/org/apache/spark/sql/execution/vectorized/WritableColumnVector.java:
##########
@@ -72,7 +72,7 @@ public void reset() {
numNulls = 0;
}
- if (hugeVectorThreshold > 0 && capacity > hugeVectorThreshold) {
Review Comment:
then shall we update the doc instead?
--
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-44239][SQL][FOLLOWUP] Do not disable vector memory optimization when hugeVectorThreshold=0 to align its document [spark]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on code in PR #47988:
URL: https://github.com/apache/spark/pull/47988#discussion_r1746400601
##########
sql/core/src/main/java/org/apache/spark/sql/execution/vectorized/WritableColumnVector.java:
##########
@@ -72,7 +72,7 @@ public void reset() {
numNulls = 0;
}
- if (hugeVectorThreshold > 0 && capacity > hugeVectorThreshold) {
Review Comment:
Changing the doc will result in code part change as other places test if hugeVectorThreshold < 0
--
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-44239][SQL][FOLLOWUP] Do not disable vector memory optimization when hugeVectorThreshold=0 to align its document [spark]
Posted by "wankunde (via GitHub)" <gi...@apache.org>.
wankunde commented on code in PR #47988:
URL: https://github.com/apache/spark/pull/47988#discussion_r1745314113
##########
sql/core/src/main/java/org/apache/spark/sql/execution/vectorized/WritableColumnVector.java:
##########
@@ -72,7 +72,7 @@ public void reset() {
numNulls = 0;
}
- if (hugeVectorThreshold > 0 && capacity > hugeVectorThreshold) {
Review Comment:
Yes， but `= 0` is not recommended, just keep code and documentation consistent
--
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-44239][SQL] Update default hugeVectorThreshold from 0 to -1 [spark]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #47988:
URL: https://github.com/apache/spark/pull/47988#issuecomment-2330509708
Hi, @dongjoon-hyun
> could be a little misleading because the default value means the following. And, this is also a code
When setting spark.sql.inMemoryColumnarStorage.hugeVectorThreshold=0, the code does not work as the doc
--
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