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

Describe show commands #533

Closed
wants to merge 18 commits into from

Conversation

YANG-DB
Copy link
Member

@YANG-DB YANG-DB commented Aug 7, 2024

Description

Support PPL Describe command

Issues Resolved

#527

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@YANG-DB YANG-DB added the Lang:PPL Pipe Processing Language support label Aug 7, 2024
@YANG-DB YANG-DB marked this pull request as draft August 7, 2024 03:43
val testTableQuoted = "`spark_catalog`.`default`.`flint_ppl_test`"
Seq(testTable, testTableQuoted).foreach { table =>
val frame = sql(s"""
describe $table
Copy link
Member

Choose a reason for hiding this comment

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

change to describe flint_ppl_test

@@ -107,6 +110,10 @@ public LogicalPlan visitExplain(Explain node, CatalystPlanContext context) {

@Override
public LogicalPlan visitRelation(Relation node, CatalystPlanContext context) {
if (node instanceof DescribeRelation) {
return context.with(new DescribeTableCommand(new TableIdentifier(node.getTableQualifiedName().toString()), null, false, seq()));
Copy link
Member

Choose a reason for hiding this comment

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

And here:

            return context.with(
                new DescribeTableCommand(
                    new TableIdentifier(node.getTableQualifiedName().toString()),
                    scala.collection.immutable.Map$.MODULE$.<String, String>empty(),
                    false,
                    DescribeRelation$.MODULE$.getOutputAttrs()));

Copy link
Member Author

@YANG-DB YANG-DB Aug 7, 2024

Choose a reason for hiding this comment

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

@LantaoJin thanks!!

@YANG-DB YANG-DB marked this pull request as ready for review August 7, 2024 05:40
Comment on lines 69 to 70
val expectedPlan: LogicalPlan =
DescribeTableCommand(TableIdentifier("default.flint_ppl_test"), null, isExtended = false, output = Seq())
Copy link
Member

Choose a reason for hiding this comment

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

@YANG-DB to make this IT test successful.
The expectedPlan should change as following either.

      val expectedPlan: LogicalPlan =
        DescribeTableCommand(
          TableIdentifier("flint_ppl_test"),
          Map.empty[String, String],
          isExtended = false,
          output = DescribeRelation.getOutputAttrs)

@YANG-DB YANG-DB requested a review from LantaoJin August 7, 2024 06:19
val testTableQuoted = "`spark_catalog`.`default`.`flint_ppl_test`"
Seq(testTable, testTableQuoted).foreach { table =>
val frame = sql(s"""
describe flint_ppl_test
Copy link
Member

Choose a reason for hiding this comment

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

Oh, we miss testing the table_identifier with quoted.

Seq("flint_ppl_test", "`flint_ppl_test`").foreach { table =>
  val frame = sql(s"""
     describe $table

Copy link
Member Author

Choose a reason for hiding this comment

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

yes probably need to add some more fail based use cases...

if (node instanceof DescribeRelation) {
return context.with(
new DescribeTableCommand(
new TableIdentifier(node.getTableQualifiedName().toString()),
Copy link
Member

@LantaoJin LantaoJin Aug 7, 2024

Choose a reason for hiding this comment

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

I checked the spark sql reference. The DESCRIBE command syntax is DESCRIBE [ database_name. ] table_name.
So to support database prefix, here need additional modification:

            TableIdentifier identifier;
            if (node.getTableQualifiedName().getParts().size() == 1) {
                identifier = new TableIdentifier(node.getTableQualifiedName().getParts().get(0));
            } else if (node.getTableQualifiedName().getParts().size() == 2) {
                identifier = new TableIdentifier(
                    node.getTableQualifiedName().getParts().get(1),
                    Option$.MODULE$.apply(node.getTableQualifiedName().getParts().get(0)));
            } else {
                throw new IllegalArgumentException("Invalid table name: " + node.getTableQualifiedName()
                    + " Syntax: [ database_name. ] table_name");
            }
            return context.with(
                new DescribeTableCommand(
                    identifier,
                    scala.collection.immutable.Map$.MODULE$.<String, String>empty(),
                    false,
                    DescribeRelation$.MODULE$.getOutputAttrs()));

Copy link
Member

@LantaoJin LantaoJin Aug 7, 2024

Choose a reason for hiding this comment

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

Then we can test on Seq("flint_ppl_test", "`flint_ppl_test`", "default.flint_ppl_test", "`default`.`flint_ppl_test`")

Copy link
Member Author

Choose a reason for hiding this comment

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

@LantaoJin thanks again

@YANG-DB YANG-DB force-pushed the describe-show-commands branch from ae97a8a to ad78b12 Compare August 8, 2024 03:16
@YANG-DB YANG-DB closed this Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Lang:PPL Pipe Processing Language support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants