Skip to content

[MINOR][SQL][DOCS] Document JDBC aggregate push down is for DSV2 only#34465

Closed
huaxingao wants to merge 2 commits intoapache:masterfrom
huaxingao:doc_minor
Closed

[MINOR][SQL][DOCS] Document JDBC aggregate push down is for DSV2 only#34465
huaxingao wants to merge 2 commits intoapache:masterfrom
huaxingao:doc_minor

Conversation

@huaxingao
Copy link
Contributor

What changes were proposed in this pull request?

To specify JDBC aggregate push down is for DS V2 only. This change is for both 3.2 and master.

Why are the changes needed?

To make the doc clear so user won't use aggregate push down in DS v1.

Does this PR introduce any user-facing change?

No. Doc change only

How was this patch tested?

Manually checked.

@SparkQA
Copy link

SparkQA commented Nov 2, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49308/

@SparkQA
Copy link

SparkQA commented Nov 2, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49308/

@SparkQA
Copy link

SparkQA commented Nov 2, 2021

Test build #144836 has finished for PR 34465 at commit 7f44f6c.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

// This only applies to Data Source V2 JDBC
val pushDownAggregate = parameters.getOrElse(JDBC_PUSHDOWN_AGGREGATE, "false").toBoolean

// An option to allow/disallow pushing down LIMIT into JDBC data source
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about pushDownLimit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pushDownLimit is also for DS v2. I will fix the doc in the pushDownSample PR I am currently working on. The reason I want a separate PR to fix JDBC aggregate push down doc is because the fix needs to be in both 3.2 and 3.3. All the other push down (pushDownLimit, pushDownSample, and aggregate push down for parquet and ORC) are for 3.3 only.

@HyukjinKwon HyukjinKwon changed the title [MINOR] Document JDBC aggregate push down is for DSV2 only [MINOR][SQL][DOCS] Document JDBC aggregate push down is for DSV2 only Nov 3, 2021
@SparkQA
Copy link

SparkQA commented Nov 3, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49355/

@SparkQA
Copy link

SparkQA commented Nov 3, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49355/

@huaxingao huaxingao closed this in 44151be Nov 3, 2021
huaxingao added a commit that referenced this pull request Nov 3, 2021
### What changes were proposed in this pull request?
To specify JDBC aggregate push down is for DS V2 only. This change is for both 3.2 and master.

### Why are the changes needed?
To make the doc clear so user won't use aggregate push down in DS v1.

### Does this PR introduce _any_ user-facing change?
No. Doc change only

### How was this patch tested?
Manually checked.

Closes #34465 from huaxingao/doc_minor.

Authored-by: Huaxin Gao <huaxin_gao@apple.com>
Signed-off-by: Huaxin Gao <huaxin_gao@apple.com>
(cherry picked from commit 44151be)
Signed-off-by: Huaxin Gao <huaxin_gao@apple.com>
@huaxingao
Copy link
Contributor Author

Merged to 3.2/master. Thanks @HyukjinKwon for reviewing!

@huaxingao huaxingao deleted the doc_minor branch November 3, 2021 21:35
@dongjoon-hyun
Copy link
Member

+1, LGTM.

@SparkQA
Copy link

SparkQA commented Nov 3, 2021

Test build #144885 has finished for PR 34465 at commit 3b8c5ef.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

sunchao pushed a commit to sunchao/spark that referenced this pull request Dec 8, 2021
### What changes were proposed in this pull request?
To specify JDBC aggregate push down is for DS V2 only. This change is for both 3.2 and master.

### Why are the changes needed?
To make the doc clear so user won't use aggregate push down in DS v1.

### Does this PR introduce _any_ user-facing change?
No. Doc change only

### How was this patch tested?
Manually checked.

Closes apache#34465 from huaxingao/doc_minor.

Authored-by: Huaxin Gao <huaxin_gao@apple.com>
Signed-off-by: Huaxin Gao <huaxin_gao@apple.com>
(cherry picked from commit 44151be)
Signed-off-by: Huaxin Gao <huaxin_gao@apple.com>
catalinii pushed a commit to lyft/spark that referenced this pull request Feb 22, 2022
### What changes were proposed in this pull request?
To specify JDBC aggregate push down is for DS V2 only. This change is for both 3.2 and master.

### Why are the changes needed?
To make the doc clear so user won't use aggregate push down in DS v1.

### Does this PR introduce _any_ user-facing change?
No. Doc change only

### How was this patch tested?
Manually checked.

Closes apache#34465 from huaxingao/doc_minor.

Authored-by: Huaxin Gao <huaxin_gao@apple.com>
Signed-off-by: Huaxin Gao <huaxin_gao@apple.com>
(cherry picked from commit 44151be)
Signed-off-by: Huaxin Gao <huaxin_gao@apple.com>
catalinii pushed a commit to lyft/spark that referenced this pull request Mar 4, 2022
### What changes were proposed in this pull request?
To specify JDBC aggregate push down is for DS V2 only. This change is for both 3.2 and master.

### Why are the changes needed?
To make the doc clear so user won't use aggregate push down in DS v1.

### Does this PR introduce _any_ user-facing change?
No. Doc change only

### How was this patch tested?
Manually checked.

Closes apache#34465 from huaxingao/doc_minor.

Authored-by: Huaxin Gao <huaxin_gao@apple.com>
Signed-off-by: Huaxin Gao <huaxin_gao@apple.com>
(cherry picked from commit 44151be)
Signed-off-by: Huaxin Gao <huaxin_gao@apple.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments