Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Use name and alias in JDBC format #932

Conversation

dai-chen
Copy link
Member

@dai-chen dai-chen commented Dec 14, 2020

Issue #, if available: #939

Description of changes:

SQL:

  1. NamedExpression: getName is renamed as getNameOrAlias. Now getName is just the getter for name.
  2. ProjectOperator always use original text (NamedExpression's name) as name in QueryResult: JDBC formatter returns both name and alias in response for client side to use.
  3. QueryResult uses name or alias as column name when iterated: Simple JSON formatter (PPL) and CSV formatter iterates QueryResult and uses it.

SQL-CLI:

  1. Use alias as table header if present in schema in response: since alias is used only when it's present, there should be no impact on PPL response.
  2. Depends on local sql-cli directly in doctest module: in this way, no need to push sql-cli artifact to PyPi manually whenever changed.

Testing:

  1. SQL: Added UT and IT. Fixed broken UT and IT.
  2. CLI's UT and IT passed:
# Build and install in virtualenv
$ cd sql-cli
$ pip install virtualenv
$ virtualenv venv
$ source ./venv/bin/activate
(venv) $ pip install --editable .

# Run tests
(venv) $ pip install -r requirements-dev.txt
(venv) $ cd tests
(venv) $ pytest
========================================================================================= test session starts ==========================================================================================
platform darwin -- Python 3.7.4, pytest-4.6.3, py-1.10.0, pluggy-0.13.1
rootdir: /Users/daichen/IdeaProjects/sql/sql-cli/tests, inifile: pytest.ini
collected 29 items

test_config.py ...                                                                                                                                                                               [ 10%]
test_esconnection.py ........                                                                                                                                                                    [ 37%]
test_formatter.py ..............                                                                                                                                                                 [ 86%]
test_main.py ..                                                                                                                                                                                  [ 93%]
test_odfesql_cli.py .s                                                                                                                                                                           [100%]

================================================================================= 28 passed, 1 skipped in 3.29 seconds =================================================================================

# Exit virtualenv
(venv) $ deactivate
$ ...

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@dai-chen dai-chen self-assigned this Dec 14, 2020
@codecov
Copy link

codecov bot commented Dec 15, 2020

Codecov Report

Merging #932 (a3e5420) into develop (ca9f578) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop     #932   +/-   ##
==========================================
  Coverage      99.86%   99.86%           
- Complexity      2334     2336    +2     
==========================================
  Files            232      232           
  Lines           5376     5379    +3     
  Branches         350      350           
==========================================
+ Hits            5369     5372    +3     
  Misses             5        5           
  Partials           2        2           
Impacted Files Coverage Δ Complexity Δ
...ndistroforelasticsearch/sql/analysis/Analyzer.java 100.00% <100.00%> (ø) 44.00 <0.00> (ø)
...rch/sql/analysis/ExpressionReferenceOptimizer.java 100.00% <100.00%> (ø) 12.00 <0.00> (ø)
...relasticsearch/sql/expression/NamedExpression.java 100.00% <100.00%> (ø) 6.00 <1.00> (ø)
...arch/sql/planner/physical/AggregationOperator.java 100.00% <100.00%> (ø) 7.00 <0.00> (ø)
...icsearch/sql/planner/physical/ProjectOperator.java 100.00% <100.00%> (ø) 7.00 <0.00> (ø)
...ge/script/aggregation/AggregationQueryBuilder.java 100.00% <100.00%> (ø) 7.00 <1.00> (ø)
...ript/aggregation/dsl/BucketAggregationBuilder.java 100.00% <100.00%> (ø) 3.00 <0.00> (ø)
...asticsearch/sql/protocol/response/QueryResult.java 100.00% <100.00%> (ø) 8.00 <3.00> (+2.00)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca9f578...a3e5420. Read the comment docs.

@dai-chen dai-chen marked this pull request as ready for review December 15, 2020 22:39
Copy link
Member

@zhongnansu zhongnansu left a comment

Choose a reason for hiding this comment

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

LGTM

@dai-chen dai-chen merged commit e1f295c into opendistro-for-elasticsearch:develop Dec 16, 2020
@dai-chen dai-chen deleted the change-name-alias-in-jdbc-format branch December 16, 2020 17:45
penghuo pushed a commit to penghuo/sql that referenced this pull request Dec 17, 2020
* Rename getName to getNameOrAlias

* Use original name as name in JDBC format

* Support alias in CLI

* Use local CLI for doctest

* Add UT

* Fix IT

* Fix IT

* Fix UT

* Update javadoc
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants