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

[Enhancement] Refactor reservedSymbolTable table. #1506

Closed
penghuo opened this issue Apr 10, 2023 · 2 comments · Fixed by #1936
Closed

[Enhancement] Refactor reservedSymbolTable table. #1506

penghuo opened this issue Apr 10, 2023 · 2 comments · Fixed by #1936
Assignees
Labels
maintenance Improves code quality, but not the product

Comments

@penghuo
Copy link
Collaborator

penghuo commented Apr 10, 2023

In OpenSearch SQL PR #1456, the reservedSymbolTable is used to store reserved symbols, such as _id. We need to explore a better approach to represent reserved symbols and examine the relationship between the reservedSymbolTable and the regular symbol table.

@penghuo penghuo added the enhancement New feature or request label Apr 10, 2023
@dai-chen dai-chen added maintenance Improves code quality, but not the product and removed enhancement New feature or request untriaged labels Apr 17, 2023
@acarbonetto
Copy link
Collaborator

Discussion tracked from #1456 (comment)

Should we include _id (and any other available reserved field) on a SELECT * FROM table request to OpenSearch?

I'd argue that we don't want this for the following reasons:

  1. Practically speaking, returning reserved fields would adversely affect a large number of IT tests and existing use cases.
  2. It would increase the response size unexpectedly, since reserved fields are not expected to return for all calls.
  3. Semantically, the table definition does not contain these fields and SQL connectors wouldn't expect anything beyond the table definition to return on wildcard requests. In other words, SELECT * should only return fields listed in the table definition.
  4. Conversely, the table definition for all tables based on OpenSearch indices should include the _ reserved words.

@acarbonetto
Copy link
Collaborator

Go with two abstractions for this, and projectoperator gets only the necessary list from the table.

@acarbonetto acarbonetto self-assigned this Aug 3, 2023
acarbonetto added a commit to Bit-Quill/opensearch-project-sql that referenced this issue Aug 3, 2023
…ELD_NAME instead

Signed-off-by: acarbonetto <andrewc@bitquilltech.com>
acarbonetto added a commit to Bit-Quill/opensearch-project-sql that referenced this issue Aug 3, 2023
…ELD_NAME instead

Signed-off-by: acarbonetto <andrewc@bitquilltech.com>
acarbonetto added a commit to Bit-Quill/opensearch-project-sql that referenced this issue Aug 3, 2023
Signed-off-by: acarbonetto <andrewc@bitquilltech.com>
acarbonetto added a commit to Bit-Quill/opensearch-project-sql that referenced this issue Aug 4, 2023
… HIDDEN_FIELD_NAME (#323)

* opensearch-project#1506: Remove reservedSymbolTable and use HIDDEN_FIELD_NAME instead

Signed-off-by: acarbonetto <andrewc@bitquilltech.com>

* opensearch-project#1506: Remove reservedSymbolTable and use HIDDEN_FIELD_NAME instead

Signed-off-by: acarbonetto <andrewc@bitquilltech.com>

* opensearch-project#1506: Fix checkstyle errors

Signed-off-by: acarbonetto <andrewc@bitquilltech.com>

---------

Signed-off-by: acarbonetto <andrewc@bitquilltech.com>
acarbonetto added a commit to Bit-Quill/opensearch-project-sql that referenced this issue Aug 4, 2023
… HIDDEN_FIELD_NAME (#323)

* opensearch-project#1506: Remove reservedSymbolTable and use HIDDEN_FIELD_NAME instead

Signed-off-by: acarbonetto <andrewc@bitquilltech.com>

* opensearch-project#1506: Remove reservedSymbolTable and use HIDDEN_FIELD_NAME instead

Signed-off-by: acarbonetto <andrewc@bitquilltech.com>

* opensearch-project#1506: Fix checkstyle errors

Signed-off-by: acarbonetto <andrewc@bitquilltech.com>

---------

Signed-off-by: acarbonetto <andrewc@bitquilltech.com>
acarbonetto added a commit to Bit-Quill/opensearch-project-sql that referenced this issue Aug 14, 2023
Signed-off-by: acarbonetto <andrewc@bitquilltech.com>
acarbonetto added a commit that referenced this issue Aug 15, 2023
…#1936)

* (#1506) Remove reservedSymbolTable and replace with HIDDEN_FIELD_NAME (#323)

* #1506: Remove reservedSymbolTable and use HIDDEN_FIELD_NAME instead

Signed-off-by: acarbonetto <andrewc@bitquilltech.com>

* #1506: Remove reservedSymbolTable and use HIDDEN_FIELD_NAME instead

Signed-off-by: acarbonetto <andrewc@bitquilltech.com>

* #1506: Fix checkstyle errors

Signed-off-by: acarbonetto <andrewc@bitquilltech.com>

---------

Signed-off-by: acarbonetto <andrewc@bitquilltech.com>

* #1506: spotless apply

Signed-off-by: acarbonetto <andrewc@bitquilltech.com>

---------

Signed-off-by: acarbonetto <andrewc@bitquilltech.com>
acarbonetto added a commit to Bit-Quill/opensearch-project-sql that referenced this issue Aug 15, 2023
… HIDDEN_FIELD_NAME (opensearch-project#1936)

* (opensearch-project#1506) Remove reservedSymbolTable and replace with HIDDEN_FIELD_NAME (#323)

* opensearch-project#1506: Remove reservedSymbolTable and use HIDDEN_FIELD_NAME instead

Signed-off-by: acarbonetto <andrewc@bitquilltech.com>

* opensearch-project#1506: Remove reservedSymbolTable and use HIDDEN_FIELD_NAME instead

Signed-off-by: acarbonetto <andrewc@bitquilltech.com>

* opensearch-project#1506: Fix checkstyle errors

Signed-off-by: acarbonetto <andrewc@bitquilltech.com>

---------

Signed-off-by: acarbonetto <andrewc@bitquilltech.com>

* opensearch-project#1506: spotless apply

Signed-off-by: acarbonetto <andrewc@bitquilltech.com>

---------

Signed-off-by: acarbonetto <andrewc@bitquilltech.com>
(cherry picked from commit 5381a6f)
Signed-off-by: acarbonetto <andrewc@bitquilltech.com>
acarbonetto added a commit that referenced this issue Aug 21, 2023
…#1936) (#1964)

* (#1506) Remove reservedSymbolTable and replace with HIDDEN_FIELD_NAME (#323)

* #1506: Remove reservedSymbolTable and use HIDDEN_FIELD_NAME instead



* #1506: Remove reservedSymbolTable and use HIDDEN_FIELD_NAME instead



* #1506: Fix checkstyle errors



---------



* #1506: spotless apply



---------


(cherry picked from commit 5381a6f)

Signed-off-by: acarbonetto <andrewc@bitquilltech.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Improves code quality, but not the product
Projects
None yet
3 participants