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.

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

[spark] Support show columns in spark #3888

Merged
merged 6 commits into from
Aug 6, 2024

Conversation

xuzifu666
Copy link
Member

@xuzifu666 xuzifu666 commented Aug 4, 2024

Purpose

Support show columns syntax in paimon,currently spark not support v2 in show columns(fix in spark apache/spark#47568 ) but paimon side not support yet,would error out like:

spark-sql> show COLUMNS from bdsp_test.paimon_mi_31;
Error in query: SHOW COLUMNS is not supported for v2 tables.

Add spark analyzer extensions to capture it,after fix:

1722744100273.png
1722861409788.png

Linked issue: #3889

Tests

API and Format

Documentation

@xuzifu666 xuzifu666 closed this Aug 4, 2024
@xuzifu666 xuzifu666 reopened this Aug 4, 2024

assertDoesNotThrow(
() => {
spark.sql("SHOW COLUMNS FROM T")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not check its result?

Copy link
Member Author

Choose a reason for hiding this comment

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

UT had added for check its result and it like picture in discription. @JingsongLi

@xuzifu666 xuzifu666 changed the title [spark] Support show columns in spark [WIP][spark] Support show columns in spark Aug 5, 2024
@xuzifu666 xuzifu666 changed the title [WIP][spark] Support show columns in spark [spark] Support show columns in spark Aug 5, 2024
import org.apache.spark.sql.catalyst.expressions.{Attribute, AttributeReference}
import org.apache.spark.sql.types.{BinaryType, Metadata, StringType}

case class PaimonShowColumnsCommand(v2Table: SparkTable)
Copy link
Contributor

Choose a reason for hiding this comment

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

this can extend PaimonLeafRunnableCommand directly, then we don't need PaimonShowColumnsExec.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks,had change it as your suggestion. @YannByron

|""".stripMargin
)

assertDoesNotThrow(
Copy link
Contributor

Choose a reason for hiding this comment

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

if check its result below, we don't need this assert.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK,had check result,I remove the assert @YannByron

@xuzifu666 xuzifu666 closed this Aug 5, 2024
@xuzifu666 xuzifu666 reopened this Aug 5, 2024
case class PaimonShowColumnsCommand(v2Table: SparkTable)
extends PaimonLeafRunnableCommand
with WithFileStoreTable {
override def table: FileStoreTable = v2Table.getTable.asInstanceOf[FileStoreTable]
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe this command can also apply to the system table, not just FileStoreTable. if yes, modify here and add a UT for that.

Copy link
Member Author

@xuzifu666 xuzifu666 Aug 6, 2024

Choose a reason for hiding this comment

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

Thanks for comments,I had a try but seems not find a way to compatible with system table and FileStoreTable due to their not use same interface. Maybe we can just support FileStoreTable firstly keep the same with other Command?

@YannByron YannByron merged commit 261393c into apache:master Aug 6, 2024
19 of 20 checks passed
wxplovecc pushed a commit to tongcheng-elong/incubator-paimon that referenced this pull request Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants