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

DRILL-6684: Swap sys.options and sys.options_val tables #1454

Merged
merged 1 commit into from
Sep 5, 2018

Conversation

kkhatua
Copy link
Contributor

@kkhatua kkhatua commented Aug 31, 2018

(As a follow up to DRILL-5735)

The current sys.options table has a verbose layout, because of which sys.options_internal was introduced. The latter supports descriptions, which means it makes sense to have that table as the new sys.options. The recommendation is to deprecate the old format, so that any dependencies continue to make use of it as long as required.
Existing tests have been patched to account for the swap.

@kkhatua
Copy link
Contributor Author

kkhatua commented Aug 31, 2018

@arina-ielchiieva / @parthchandra can you please review this?

@khurram-faraaz
Copy link

@kkhatua Is it not possible to add descriptions to the existing sys.options table ?
Will users no longer be able to query from sys.options table, with this new change ?

@kkhatua
Copy link
Contributor Author

kkhatua commented Aug 31, 2018

@kfaraaz
The existing sys.options table is very verbose because of multiple 'datatype_val' columns (all except one will be alway null). Injecting a descriptions column breaks the visual formatting within SQLLine.
The existing sys.options_val is better representative of the options. For usability sake, we are renaming the latter table as sys.options , while the old table becomes sys.options_old. So, both tables will still be available.

Copy link
Member

@arina-ielchiieva arina-ielchiieva left a comment

Choose a reason for hiding this comment

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

Looks good, one minor comment though.

The current `sys.options` table has a verbose layout, because of which `sys.options_internal` was introduced. The latter also supports descriptions, which means it makes sense to have that table as the new `sys.options`. 
This PR deprecates the old format, so that any dependencies continue to make use of it as long as required.
@kkhatua kkhatua merged commit 14ad838 into apache:master Sep 5, 2018
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.

None yet

3 participants