Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[SPARK-36895][SQL][FOLLOWUP] Use property to specify index type #34523

Closed
wants to merge 1 commit into from
Closed

[SPARK-36895][SQL][FOLLOWUP] Use property to specify index type #34523

wants to merge 1 commit into from

Conversation

huaxingao
Copy link
Contributor

What changes were proposed in this pull request?

use property to specify index type

Why are the changes needed?

fix scala doc in #34486 and resubmit

Does this PR introduce any user-facing change?

Yes

  void createIndex(String indexName,
      String indexType,
      NamedReference[] columns,
      Map<NamedReference, Map<String, String>> columnsProperties,
      Map<String, String> properties)

changed to

createIndex(String indexName,
      NamedReference[] columns,
      Map<NamedReference, Map<String, String>> columnsProperties,
      Map<String, String> properties

How was this patch tested?

new test

@github-actions github-actions bot added the SQL label Nov 8, 2021
@SparkQA
Copy link

SparkQA commented Nov 8, 2021

Test build #145007 has started for PR 34523 at commit 33a2c7a.

sarutak
sarutak reviewed Nov 8, 2021
View reviewed changes
sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala
@@ -295,7 +295,6 @@ abstract class JdbcDialect extends Serializable with Logging{
* Build a create index SQL statement.
*
* @param indexName the name of the index to be created
* @param indexType the type of the index to be created
Copy link
Member

Choose a reason for hiding this comment

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

@huaxingao The difference from #34486 is only this part right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. Sorry I forgot to delete this last time.

sarutak
sarutak approved these changes Nov 8, 2021
View reviewed changes
Copy link
Member

@sarutak sarutak left a comment

Choose a reason for hiding this comment

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

LGTM. Pending CIs.

@SparkQA
Copy link

SparkQA commented Nov 8, 2021

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49480/

@sarutak
Copy link
Member

sarutak commented Nov 8, 2021

The difference from #34486 is only this part and GA (and generating JavaDoc) seems to have passed so I'll merge this.
Thank you @huaxingao !

@sarutak sarutak closed this in d8a4a8c Nov 8, 2021
@huaxingao
Copy link
Contributor Author

Thank you very much! @sarutak

@huaxingao huaxingao deleted the newDelete branch November 8, 2021 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sarutak sarutak sarutak approved these changes

Assignees
No one assigned
Labels
SQL
Projects
None yet
Milestone
No milestone
3 participants
@huaxingao @SparkQA @sarutak