Skip to content

Conversation

@shivusondur
Copy link
Contributor

@shivusondur shivusondur commented Aug 23, 2019

What changes were proposed in this pull request?

Added the reference for SHOW TABLES sql command.

Why are the changes needed?

To help the customer usage

Does this PR introduce any user-facing change?

It updates the Sql command reference doc.

How was this patch tested?

Attached the Snap

image

image

Copy link
Contributor

Choose a reason for hiding this comment

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

form -> from.

Copy link
Contributor

Choose a reason for hiding this comment

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

databse -> database.

Copy link
Contributor

Choose a reason for hiding this comment

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

refer tables.scala for syntax

@shivusondur
Copy link
Contributor Author

@dilipbiswal @gatorsmile
Updated the PR according to #25525 and #25523 PR's.
Updated the ScreenShot after creating the document from Jekyll build

Please review and kindly check for merging.

@gatorsmile
Copy link
Member

ok to test

@SparkQA
Copy link

SparkQA commented Sep 20, 2019

Test build #111092 has finished for PR 25561 at commit 172849a.

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

</dd>
<dt><code><em>LIKE 'regex_pattern'</em></code></dt>
<dd>
A regex pattern that is used to filter out unwanted functions.
Copy link
Contributor

Choose a reason for hiding this comment

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

@shivusondur Can you please check the code here ? Looks like this is copied from SHOW FUNCTIONS. Please make sure the pattern is processed the same way in SHOW TABLES. If so, you can change from "unwanted functions" to "unwanted tables"..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Handled

{% highlight sql %}
-- List all tables in default database
SHOW TABLES;
+-----------+------------+--------------+--+
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: two space align for results to match the other prs ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Handled


{% endhighlight %}

### Related statements.
Copy link
Contributor

Choose a reason for hiding this comment

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

remove period

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

SHOW TABLES [{FROM|IN} database_name] [LIKE 'regex_pattern']
{% endhighlight %}

### Parameter
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Parameters ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

2. Changed to  "unwanted tables"
3. Added the "two space align in the result"
4. removed the period in "statements"
5. verified the links in the Related statements
@shivusondur
Copy link
Contributor Author

@dilipbiswal
i handled all your comments, please have a look.
and again thanks for your review.

@SparkQA
Copy link

SparkQA commented Sep 21, 2019

Test build #111117 has finished for PR 25561 at commit 19f39fc.

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

<dt><code><em>LIKE 'regex_pattern'</em></code></dt>
<dd>
Specifies the regex pattern that is used to filter out unwanted tables.
<br> - Only `*` and `|` are allowed as wildcard pattern.
Copy link
Contributor

Choose a reason for hiding this comment

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

@shivusondur Can you please include the example with output using the 'I' as wildcard ? From the code its not clear to me if this is supported.

Copy link
Contributor Author

@shivusondur shivusondur Sep 22, 2019

Choose a reason for hiding this comment

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

{% highlight sql %}
-- List all tables in default database
SHOW TABLES;
+-----------+------------+--------------+--+
Copy link
Contributor

@dilipbiswal dilipbiswal Sep 21, 2019

Choose a reason for hiding this comment

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

Please lets fix the formatting. Did we construct this output by hand ? Can we paste the
actual output here please so we preserve the formatting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dilipbiswal
I just Copy and paste the actual output from the Beeline console
Here is the beeline console snap
image

In the UI it looks like below, i think it is normal
image

Only in the git file editor, it shows inconsistent.

@SparkQA
Copy link

SparkQA commented Sep 22, 2019

Test build #111140 has finished for PR 25561 at commit 55ea2cf.

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

| default | sam | false |
| default | sam1 | false |
+-----------+------------+--------------+--+
-- List all tables matching the pattern `sam*|suj`
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: can we have a line between previous test and this one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dilipbiswal
Done, updated the snap's also.

@SparkQA
Copy link

SparkQA commented Sep 22, 2019

Test build #111148 has finished for PR 25561 at commit 0dd4efb.

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

### Description

**This page is under construction**
List all the `tables` from the database with `database-name` and `is temporary table`.
Copy link
Contributor

Choose a reason for hiding this comment

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

@shivusondur Sorry.. re-reading this one last time :-)
SHOW TABLES statement returns all the tables for an optionally specified database. Additionally, the output
of this statement may be filtered via an optional matching pattern. If no database is specified then the tables
are returned from the current database.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DOne

@dilipbiswal
Copy link
Contributor

LGTM - except a minor comment.

@dilipbiswal
Copy link
Contributor

cc @gatorsmile

@shivusondur
Copy link
Contributor Author

@dilipbiswal and @gatorsmile
Handled the all the comments.

@SparkQA
Copy link

SparkQA commented Sep 23, 2019

Test build #111176 has finished for PR 25561 at commit 1d08b22.

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

<dt><code><em>LIKE 'regex_pattern'</em></code></dt>
<dd>
Specifies the regex pattern that is used to filter out unwanted tables.
<br> - Only `*` and `|` are allowed as wildcard pattern.
Copy link
Member

Choose a reason for hiding this comment

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

| isn't a wildcard pattern, so I wouldn't imply it is.
Is it really treated as a full regex otherwise? if not, I wouldn't call it a regex unless it is generally called this in other similar references.

Copy link
Contributor

Choose a reason for hiding this comment

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

@srowen Thats a very good point Sean. How about we name the pattern 'identifier_with_wildcards' (and describe as such) as mentioned in ShowTablesCommand's comment ?

Copy link
Member

Choose a reason for hiding this comment

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

Is it a SQL-like pattern that supports wildcards like %? then I'd just call it a pattern

Copy link
Contributor

Choose a reason for hiding this comment

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

@srowen I quickly tried in my env. It does not support SQL-like pattern like %. I only saw the following work :

  • * : Matches 0 or more characters.
  • . : Matches a char
  • | : works like or expression.

@shivusondur Can you please confirm ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dilipbiswal @srowen
I also tesed, it is same behaviour you mentioned above.
It will use the below method to handle, here they are handling "|" and "*" seperatly, remaining treating as regex

def filterPattern(names: Seq[String], pattern: String): Seq[String] = {

Copy link
Member

Choose a reason for hiding this comment

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

OK, your comment is consistent with the docs then. Hm, but isn't it more accurate to say that * and | don't work like in regexes? * matches 0 or more characters, and | delimits whole regexes, which is different, but the rest is the same. Maybe a comment more like that. "The pattern is a regex except that * matches 0 or more characters by itself, and | may only be used at the top level to delimit alternative regexes to match." ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@srowen @dilipbiswal
Thank you both for your time in reviewing.

I updated according to your suggestion and screenshot also I updated.
Now it will look like below.
image

@SparkQA
Copy link

SparkQA commented Oct 1, 2019

Test build #111655 has finished for PR 25561 at commit 847fb8e.

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

### Description

**This page is under construction**
`SHOW TABLES` statement returns all the tables for an optionally specified `database`.
Copy link
Member

Choose a reason for hiding this comment

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

Nit: add "The" at the start.
via -> by

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@shivusondur shivusondur requested a review from srowen October 3, 2019 18:00
@SparkQA
Copy link

SparkQA commented Oct 3, 2019

Test build #111750 has finished for PR 25561 at commit 05a2e60.

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

<dt><code><em>LIKE 'regex_pattern'</em></code></dt>
<dd>
Specifies the regex pattern that is used to filter out unwanted tables.
<br> - The pattern is a regex except that `*` and `|`characters
Copy link
Member

Choose a reason for hiding this comment

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

"... except that * and | work differently"

### Description

**This page is under construction**
The `SHOW TABLES` statement returns all the tables for an optionally specified `database`.
Copy link
Member

Choose a reason for hiding this comment

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

Nit: no need to back-tick database here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@srowen
Done

<dt><code><em>LIKE 'regex_pattern'</em></code></dt>
<dd>
Specifies the regex pattern that is used to filter out unwanted tables.
<br> - The pattern is a regex except that `*` and `|`characters
Copy link
Contributor

Choose a reason for hiding this comment

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

@shivusondur drop the "that" ?
@srowen Can you help check the above sentence please ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SparkQA
Copy link

SparkQA commented Oct 7, 2019

Test build #111830 has finished for PR 25561 at commit fed9431.

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

@dilipbiswal
Copy link
Contributor

LGTM
cc @srowen @gatorsmile

<dt><code><em>LIKE 'regex_pattern'</em></code></dt>
<dd>
Specifies the regex pattern that is used to filter out unwanted tables.
<br> - The pattern is a regex except `*` and `|`characters
Copy link
Member

Choose a reason for hiding this comment

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

Nit: can you use an HTML unordered list? <ul><li>... etc? Or just make this a paragraph, not bullet points.

I still think this is kind of unclear, per the last comment. "except * ..." should be clarified. "except that * and | have a different meaning."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.
used the html ul and li tags
and added the description like below
Except *and| characters remaining characters will follow the regular expression convention.

@shivusondur shivusondur requested a review from srowen October 9, 2019 04:41
<br> - The leading and trailing blanks are trimmed in the input pattern before processing.
Specifies the regular expression pattern that is used to filter out unwanted tables.
<ul>
<li> Except `*` and `|` characters remaining characters will follow the regular expression convention.</li>
Copy link
Member

Choose a reason for hiding this comment

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

"Except for * and |, the pattern works like a regex"

Specifies the regular expression pattern that is used to filter out unwanted tables.
<ul>
<li> Except `*` and `|` characters remaining characters will follow the regular expression convention.</li>
<li> `*` matches 0 or more characters and `|` used to provide more than one regex with OR condition. </li>
Copy link
Member

Choose a reason for hiding this comment

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

"* alone matches 0 or more characters, and | is used to separate multiple different regexes, any of which can match"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

comments handled
Thanks for reviewing

@shivusondur shivusondur requested a review from srowen October 10, 2019 04:06
<dd>
Specifies the regular expression pattern that is used to filter out unwanted tables.
<ul>
<li> Except for * and | character, the pattern works like a regex.</li>
Copy link
Member

Choose a reason for hiding this comment

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

OK, I might put back the back-ticks or quotes around special chars here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. The same correction I also interested. thank you

@shivusondur shivusondur requested a review from srowen October 11, 2019 03:52
@srowen srowen closed this in aa1acfe Oct 12, 2019
@srowen
Copy link
Member

srowen commented Oct 12, 2019

Merged to master

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.

8 participants