Adding SET statement for configuration management #1180

hershd23 · 2023-09-21T00:50:34Z

Added

Parser for SET statement
Class for SET statement
StatementType.SET to parser.types

modified: evadb/parser/evadb.lark

modified: evadb/parser/evadb.lark Added class for set_statement new file: evadb/parser/set_statement.py Added SET to statement_types modified: evadb/parser/types.py

statement modified: evadb/parser/evadb.lark modified: evadb/parser/lark_visitor/__init__.py new file: evadb/parser/lark_visitor/_set_statement.py

modified: evadb/executor/plan_executor.py new file: evadb/executor/set_executor.py Added an optimizer for set queries modified: evadb/optimizer/operators.py modified: evadb/optimizer/statement_to_opr_converter.py Added a plan for set queries new file: evadb/plan_nodes/set_plan.py modified: evadb/plan_nodes/types.py

hershd23 · 2023-09-21T02:57:03Z

Hi @jiashenC @xzdandy @gaurav274 adding you as reviewers. This PR is up for an early review

Have added parser, executor, plan and optimizer for SET queries please give them a look.

I see that the we send config with a category param as well. Wanted to make sure if we wanted to continue with that design, since in the example given duckDB did not have that
https://duckdb.org/docs/sql/configuration.html

Will take up SELECT command for displaying configs and tests for this in separate PRs

jiashenC

Thanks! The design looks good to me. Just some clarification questions

Are we going to universally use string type for config value? This can be easier if we want to store those values in the catalog.
Are we going to persist configuration values and those changes will be part of this PR?

hershd23 · 2023-09-21T04:23:53Z

For 1, I am not very sure since there is an example where we are adding tuples in config_value. Example -

evadb/benchmark/text_summarization/text_summarization_with_evadb.py

Line 28 in 08b7e4c

cursor._evadb.config.update_value("executor", "gpu_ids", [0,1])

.

We could serialize-deserialize but that would also require us saving the type.

For 2, I do not think we are. This is more about about creating the query to manage configurations. Persisting to disk can be taken as a natural extension of this problem once done.

evadb/executor/set_executor.py

evadb/parser/evadb.lark

xzdandy · 2023-09-21T05:58:51Z

Great work. Looks good to me overall. Let's add a test cases to verify the config value has been changed.

hershd23 · 2023-09-22T03:46:40Z

Hi @jiashenC, @xzdandy and @gaurav274. Was confused about this and wanted to run this by you.

In eva the current way we do configs is by three params

Category
config_key
config_value

This PR captures config_key and config_value but not category. For that do we want to change the method to include category or not?

If we change category then our query looks something like

SET EXECUTOR GPU_IDS = [0, 1]

OR

SET GPU_IDS = [0, 1] IN EXECUTOR

or something that involves passing the category as well.

OR

Since DuckDB and Postgres like @xzdandy added they just have key and value. No concept of category. That is the way query is currently implemented. This seems to be the common way DBs add configs.

SET GPU_IDS = [0, 1]

What do you say?

jiashenC · 2023-09-22T03:51:00Z

Hi @jiashenC, @xzdandy and @gaurav274. Was confused about this and wanted to run this by you.

In eva the current way we do configs is by three params
1. Category

2. config_key

3. config_value
This PR captures config_key and config_value but not category. For that do we want to change the method to include category or not?

If we change category then our query looks something like
SET EXECUTOR GPU_IDS = [0, 1]

OR

SET GPU_IDS = [0, 1] IN EXECUTOR
or something that involved passing the category as well.

OR

Since DuckDB and Postgres like @xzdandy added they just have key and value. No concept of category. That is the way query is currently implemented. This seems to be the common way DBs add configs.
SET GPU_IDS = [0, 1]
What do you say?

I vote for going with Postgres's approach.

jarulraj · 2023-09-22T03:53:08Z

Let's ditch category

https://www.postgresql.org/docs/current/config-setting.html#CONFIG-SETTING-SQL

modified: evadb/executor/set_executor.py Since we use a comparitor in the LOGICAL OPERTATORS on LOGICAL DELIMITER, we have to put LOGICALSET over LOGICALDELIMITER modified: evadb/optimizer/operators.py Fixed typo (private data member) modified: evadb/optimizer/statement_to_opr_converter.py Added a construct for TO modified: evadb/parser/evadb.lark Make class variables as properties modified: evadb/parser/set_statement.py Added a short integration test for end to end query testing new file: test/integration_tests/short/test_set_executor.py Added a unit test for SET query parsing modified: test/unit_tests/parser/test_parser.py

modified: evadb/executor/plan_executor.py modified: evadb/executor/set_executor.py modified: evadb/parser/utils.py modified: test/unit_tests/parser/test_parser.py

modified: evadb/executor/set_executor.py modified: evadb/plan_nodes/set_plan.py

hershd23 · 2023-09-22T20:34:34Z

@gaurav274 @xzdandy @jiashenC this is up for review

xzdandy

Great work. Nits: fix the linter.

hershd23 · 2023-09-22T20:56:19Z

Done @xzdandy

hershd23 · 2023-09-22T21:19:00Z

Since I am not using the plan classes now, I will remove them

skipping those modified: evadb/executor/set_executor.py modified: evadb/optimizer/operators.py modified: evadb/optimizer/statement_to_opr_converter.py deleted: evadb/plan_nodes/set_plan.py modified: evadb/plan_nodes/types.py modified: test/unit_tests/optimizer/test_statement_to_opr_converter.py

hershd23 added 5 commits September 18, 2023 11:42

Adding SET statement to parser

0703cc4

modified: evadb/parser/evadb.lark

Added parser for set_statement

d1a092b

modified: evadb/parser/evadb.lark Added class for set_statement new file: evadb/parser/set_statement.py Added SET to statement_types modified: evadb/parser/types.py

Merge branch 'staging' into configuration_query

e1b7b10

Added lark interpreter and added config_name and config_value to lark

bf5c30a

statement modified: evadb/parser/evadb.lark modified: evadb/parser/lark_visitor/__init__.py new file: evadb/parser/lark_visitor/_set_statement.py

Fixed unit and short integration tests

4653edb

hershd23 requested review from jiashenC, gaurav274 and xzdandy September 21, 2023 03:35

jiashenC reviewed Sep 21, 2023

View reviewed changes

xzdandy reviewed Sep 21, 2023

View reviewed changes

evadb/executor/set_executor.py Outdated Show resolved Hide resolved

xzdandy reviewed Sep 21, 2023

View reviewed changes

evadb/parser/evadb.lark Outdated Show resolved Hide resolved

xzdandy linked an issue Sep 21, 2023 that may be closed by this pull request

Configuration Manager Redesign #1140

Closed

5 tasks

xzdandy assigned hershd23 Sep 21, 2023

xzdandy added the Feature Request ✨ New feature or request label Sep 21, 2023

xzdandy added this to the v0.3.7 milestone Sep 22, 2023

xzdandy added User Experience and removed Feature Request ✨ New feature or request labels Sep 22, 2023

hershd23 added 3 commits September 22, 2023 10:44

Lint changes

22ae18f

Merge branch 'staging' into configuration_query

b52cfc7

hershd23 mentioned this pull request Sep 22, 2023

Configuration Manager Redesign #1140

Closed

5 tasks

hershd23 added 2 commits September 22, 2023 16:18

Moved SetStatement to the list of SKIP_BINDER_AND_OPTIMIZER_STATEMENTS

d19c1e5

modified: evadb/executor/plan_executor.py modified: evadb/executor/set_executor.py modified: evadb/parser/utils.py modified: test/unit_tests/parser/test_parser.py

Fixed linter and unit test cases

b7ee3b6

modified: evadb/executor/set_executor.py modified: evadb/plan_nodes/set_plan.py

xzdandy approved these changes Sep 22, 2023

View reviewed changes

Resolved linter issues

182aca4

xzdandy removed a link to an issue Sep 23, 2023

Configuration Manager Redesign #1140

Closed

5 tasks

xzdandy merged commit 27e2635 into georgia-tech-db:staging Sep 23, 2023

xzdandy linked an issue Sep 23, 2023 that may be closed by this pull request

Configuration Manager Redesign #1140

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding SET statement for configuration management #1180

Adding SET statement for configuration management #1180

hershd23 commented Sep 21, 2023 •

edited

Loading

hershd23 commented Sep 21, 2023

jiashenC left a comment

hershd23 commented Sep 21, 2023 •

edited

Loading

xzdandy commented Sep 21, 2023

hershd23 commented Sep 22, 2023 •

edited

Loading

jiashenC commented Sep 22, 2023 •

edited

Loading

jarulraj commented Sep 22, 2023

hershd23 commented Sep 22, 2023

xzdandy left a comment •

edited

Loading

hershd23 commented Sep 22, 2023

hershd23 commented Sep 22, 2023

Adding SET statement for configuration management #1180

Adding SET statement for configuration management #1180

Conversation

hershd23 commented Sep 21, 2023 • edited Loading

hershd23 commented Sep 21, 2023

jiashenC left a comment

Choose a reason for hiding this comment

hershd23 commented Sep 21, 2023 • edited Loading

xzdandy commented Sep 21, 2023

hershd23 commented Sep 22, 2023 • edited Loading

jiashenC commented Sep 22, 2023 • edited Loading

jarulraj commented Sep 22, 2023

hershd23 commented Sep 22, 2023

xzdandy left a comment • edited Loading

Choose a reason for hiding this comment

hershd23 commented Sep 22, 2023

hershd23 commented Sep 22, 2023

hershd23 commented Sep 21, 2023 •

edited

Loading

hershd23 commented Sep 21, 2023 •

edited

Loading

hershd23 commented Sep 22, 2023 •

edited

Loading

jiashenC commented Sep 22, 2023 •

edited

Loading

xzdandy left a comment •

edited

Loading