Skip to content

SQL: drop BASE TABLE type in favour for just TABLE#54836

Merged
bpintea merged 6 commits intoelastic:masterfrom
bpintea:enh/drop_base_table
Apr 8, 2020
Merged

SQL: drop BASE TABLE type in favour for just TABLE#54836
bpintea merged 6 commits intoelastic:masterfrom
bpintea:enh/drop_base_table

Conversation

@bpintea
Copy link
Copy Markdown
Contributor

@bpintea bpintea commented Apr 6, 2020

This PR drops the table type BASE TABLE for indices and replaces
all occurrences with just TABLE, since his type is widely-used and
friendlier to the client applications that query for certain table types
in their discovery mode.

The TABLE type is also explicitly mentioned by the JDBC and ODBC
standards and although other data source-specific types are permitted,
older apps will not work well with them.

bpintea added 2 commits April 6, 2020 20:39
This commit drops the table type 'BASE TABLE' and replaces all
occurences with just 'TABLE', since his type is wider-used and
friendlier to the client applications that query for certain table types
in their discovery mode.

The 'TABLE' type is also explicitely mentioned by the JDBC and ODBC
standards and although other data source-specific types are permitted,
older apps will not work well with them.
Update tests, rename 'BASE TABLE' to 'TABLE'.
@bpintea
Copy link
Copy Markdown
Contributor Author

bpintea commented Apr 7, 2020

@elasticmachine update branch

@bpintea
Copy link
Copy Markdown
Contributor Author

bpintea commented Apr 7, 2020

@elasticmachine update branch

@bpintea
Copy link
Copy Markdown
Contributor Author

bpintea commented Apr 7, 2020

@elasticmachine run elasticsearch-ci/bwc

@bpintea
Copy link
Copy Markdown
Contributor Author

bpintea commented Apr 7, 2020

@elasticmachine update branch

@bpintea bpintea marked this pull request as ready for review April 7, 2020 16:54
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-ql (:Query Languages/SQL)

@bpintea bpintea requested review from astefan, costin and matriv April 7, 2020 16:55
Copy link
Copy Markdown
Contributor

@matriv matriv left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Member

@costin costin left a comment

Choose a reason for hiding this comment

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

LGTM

UNKNOWN("UNKNOWN", "UNKNOWN");

public static final String SQL_BASE_TABLE = "BASE TABLE";
public static final String SQL_TABLE = "TABLE";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since these constants are used somewhere else, it makes sense to move them outside IndexType and reuse them inside the enum declaration as well.

executeCommand("SYS TABLES CATALOG LIKE '' LIKE '' ", r -> {
assertEquals(2, r.size());
assertEquals("BASE TABLE", r.column(3));
assertEquals(SQL_TABLE, r.column(3));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

Copy link
Copy Markdown
Contributor

@astefan astefan left a comment

Choose a reason for hiding this comment

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

LGTM

Move SQL_TABLE/_ALIAS out of IndexType, so that they can also be used in
that Enum definition.
@bpintea
Copy link
Copy Markdown
Contributor Author

bpintea commented Apr 8, 2020

@elasticmachine run elasticsearch-ci/bwc elasticsearch-ci/2

@bpintea
Copy link
Copy Markdown
Contributor Author

bpintea commented Apr 8, 2020

@elasticmachine run elasticsearch-ci/2

@bpintea bpintea merged commit 70241b5 into elastic:master Apr 8, 2020
@bpintea bpintea deleted the enh/drop_base_table branch April 8, 2020 12:46
bpintea added a commit to bpintea/elasticsearch that referenced this pull request Apr 8, 2020
* Drop BASE TABLE type in favour for just TABLE

This commit drops the table type 'BASE TABLE' and replaces all
occurences with just 'TABLE', since his type is wider-used and
friendlier to the client applications that query for certain table types
in their discovery mode.

The 'TABLE' type is also explicitely mentioned by the JDBC and ODBC
standards and although other data source-specific types are permitted,
older apps will not work well with them.

* Refactor table type constants out of IndexType

Move SQL_TABLE/_ALIAS out of IndexType, so that they can also be used in
that Enum definition.

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
(cherry picked from commit 70241b5)
bpintea added a commit that referenced this pull request Apr 8, 2020
* Drop BASE TABLE type in favour for just TABLE

This commit drops the table type 'BASE TABLE' and replaces all
occurences with just 'TABLE', since his type is wider-used and
friendlier to the client applications that query for certain table types
in their discovery mode.

The 'TABLE' type is also explicitely mentioned by the JDBC and ODBC
standards and although other data source-specific types are permitted,
older apps will not work well with them.

* Refactor table type constants out of IndexType

Move SQL_TABLE/_ALIAS out of IndexType, so that they can also be used in
that Enum definition.

(cherry picked from commit 70241b5)
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.

6 participants