PHOENIX-5543: Implement SHOW TABLES/SCHEMAS sql commands #606

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

Merged

gjacoby126 merged 1 commit into apache:master from bharathv:PHOENIX-5543

May 7, 2020

Contributor

bharathv commented Oct 24, 2019

This patch adds new SQL grammar like following

SHOW SCHEMAS [like '']
SHOW TABLES [IN ] [like '<pattern']

Example invocations:

show schemas
show scemas like 'SYS%'
show tables
show tables in SYSTEM
show tables in SYSTEM like 'CAT%'

The current way of fetching this information is by using
!tables and !schemas via sqlline JDBC support but that is
not flexible enough for the end users to add more fitlers.
This approach is more inline with what other databases do.

Added test coverage in parser tests and core e2e tests.

Contributor Author

bharathv commented Oct 25, 2019

@vincentpoon FYI.

yanxinyi reviewed

View reviewed changes

phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixDatabaseMetaData.java Outdated Show resolved Hide resolved

yanxinyi reviewed

View reviewed changes

phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificTablesDDLIT.java Outdated Show resolved Hide resolved

Contributor

yanxinyi commented Nov 1, 2019

+1 from my side.

yanxinyi reviewed

View reviewed changes

phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificTablesDDLIT.java Outdated

    
                  public void testShowTablesMultiTenant() throws Exception {

                      // Each tenant should only be able list tables corresponding to their TENANT_ID

                      String tenantId2 = "T_" + generateUniqueName();

                      String secondTenantConnectionURL = PHOENIX_JDBC_TENANT_SPECIFIC_URL.replace(TENANT_ID,  tenantId2);

Contributor

yanxinyi Nov 1, 2019

nit: more than 100 char per line

Contributor Author

bharathv Apr 14, 2020

Done.

gjacoby126 approved these changes

View reviewed changes

Contributor

gjacoby126 left a comment

+1. (Sorry for the long wait!) When you get the chance, please rebase and fix the one tiny style nit around while loops.

phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificTablesDDLIT.java Outdated

    
                      try (Connection conn = DriverManager.getConnection(getUrl(), props)) {

                          Set<String> tables = new HashSet<>();

                          ResultSet rs = conn.prepareStatement("show tables").executeQuery();

                          while (rs.next()) tables.add(rs.getString("TABLE_NAME"));

Contributor

gjacoby126 Oct 28, 2019

nit: please put loop body in brackets.

Contributor Author

bharathv Apr 14, 2020

Done.

phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificTablesDDLIT.java Outdated

    
                      try (Connection conn = DriverManager.getConnection(secondTenantConnectionURL, props)) {

                          Set<String> tables = new HashSet<>();

                          ResultSet rs = conn.prepareStatement("show tables").executeQuery();

                          while (rs.next()) tables.add(rs.getString("TABLE_NAME"));

Contributor

gjacoby126 Apr 14, 2020

ditto for loop in brackets

Contributor Author

bharathv Apr 14, 2020

Done.

bharathv force-pushed the PHOENIX-5543 branch from a415e98 to ce21978 Compare

April 14, 2020 22:14

bharathv commented

View reviewed changes

Contributor Author

bharathv left a comment

Addressed the comments, thanks for taking a look.

phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificTablesDDLIT.java Outdated

    
                  public void testShowTablesMultiTenant() throws Exception {

                      // Each tenant should only be able list tables corresponding to their TENANT_ID

                      String tenantId2 = "T_" + generateUniqueName();

                      String secondTenantConnectionURL = PHOENIX_JDBC_TENANT_SPECIFIC_URL.replace(TENANT_ID,  tenantId2);

Contributor Author

bharathv Apr 14, 2020

Done.

phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificTablesDDLIT.java Outdated

    
                      try (Connection conn = DriverManager.getConnection(getUrl(), props)) {

                          Set<String> tables = new HashSet<>();

                          ResultSet rs = conn.prepareStatement("show tables").executeQuery();

                          while (rs.next()) tables.add(rs.getString("TABLE_NAME"));

Contributor Author

bharathv Apr 14, 2020

Done.

phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificTablesDDLIT.java Outdated

    
                      try (Connection conn = DriverManager.getConnection(secondTenantConnectionURL, props)) {

                          Set<String> tables = new HashSet<>();

                          ResultSet rs = conn.prepareStatement("show tables").executeQuery();

                          while (rs.next()) tables.add(rs.getString("TABLE_NAME"));

Contributor Author

bharathv Apr 14, 2020

Done.

bharathv commented

View reviewed changes

Contributor Author

bharathv left a comment

Attached a patch to the jira to kick-off a pre-commit build. It looks like it doesn't happen automatically on PRs.

liuml07 reviewed

View reviewed changes

Member

liuml07 left a comment

Nice feature. But I don't have enough context to be +1. Thanks,

phoenix-core/src/main/java/org/apache/phoenix/parse/ShowSchemasStatement.java

    
                  }

                  @Override

                  public boolean equals(Object other) {

Member

liuml07 Apr 14, 2020

Since you are overriding equals, I would say let's also do the hashCode(). Same to ShowTablesStatement.

Contributor Author

bharathv May 7, 2020

Not sure if thats of much use, but I still added it.

Member

liuml07 May 7, 2020 •

edited

Loading

You want to check Effective Java item 11?

If this class is an exception, we at least should put a comment saying why overriding equals() here does not need to override hasCode().

phoenix-core/src/main/java/org/apache/phoenix/parse/ShowSchemasStatement.java Outdated

    
               */

              public class ShowSchemasStatement extends ShowStatement {

                  @Nullable

                  private String schemaPattern;

Member

liuml07 Apr 14, 2020

final so this class is immutable. Not sure if we need it to be immutable today, but it's nice when we need it someday in future. Same to ShowTablesStatement.

Contributor Author

bharathv May 7, 2020

Done.

phoenix-core/src/main/java/org/apache/phoenix/parse/ShowSchemasStatement.java Outdated

    
                  public void toSQL(ColumnResolver resolver, StringBuilder buf) {

                      Preconditions.checkNotNull(buf);

                      buf.append("SHOW SCHEMAS ");

Member

liuml07 Apr 14, 2020

	buf.append("SHOW SCHEMAS ");
        if (schemaPattern != null) {
            buf.append("LIKE ");

to

	buf.append("SHOW SCHEMAS");
        if (schemaPattern != null) {
            buf.append(" LIKE ");

Not sure if this makes sense.

Contributor Author

bharathv May 7, 2020

Done.

bharathv force-pushed the PHOENIX-5543 branch from ce21978 to 2176798 Compare

May 7, 2020 00:41

gjacoby126 reviewed

View reviewed changes

phoenix-core/src/main/java/org/apache/phoenix/parse/ShowTablesStatement.java

    
                  @Override

                  public int hashCode() {

                    return Objects.hash(targetSchema, dbPattern);

Contributor

gjacoby126 May 7, 2020 •

edited

Loading

nit: the Java standard says that if two objects are equal, they have to have the same hash code (https://docs.oracle.com/javase/7/docs/api/java/lang/Object.html) but here we judge equality purely by targetSchema and hashCode by both targetSchema and dbPattern.

Contributor

gjacoby126 May 7, 2020

After that I think this is ready for me to commit unless anyone else has any comments.

Contributor Author

bharathv May 7, 2020

That was an oversight on my part, fixed.

liuml07 approved these changes

View reviewed changes

Member

liuml07 commented May 7, 2020 •

edited

Loading

looks good to me based on Geoffrey's comment. Thanks for updating


          PHOENIX-5543: Implement SHOW TABLES/SCHEMAS sql commands

4bea08b

This patch adds new SQL grammar like following

- SHOW SCHEMAS [like '<pattern>']
- SHOW TABLES [IN <schema>] [like '<pattern']

Example invocations:

- show schemas
- show scemas like 'SYS%'
- show tables
- show tables in SYSTEM
- show tables in SYSTEM like 'CAT%'

The current way of fetching this information is by using
!tables and !schemas via sqlline JDBC support but that is
not flexible enough for the end users to add more fitlers.
This approach is more inline with what other databases do.

Added test coverage in parser tests and core e2e tests.

bharathv force-pushed the PHOENIX-5543 branch from 2176798 to 4bea08b Compare

May 7, 2020 06:40

gjacoby126 merged commit 116b9bd into apache:master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet