datafusion icon indicating copy to clipboard operation
datafusion copied to clipboard

Improve doc on lowercase treatment of columns on SQL

Open nanicpc opened this issue 3 years ago • 2 comments

Adds specific note on lowercase treatment of columns on SQL

Which issue does this PR close?

Closes #2374

Rationale for this change

This example is the first thing newcomers see when they start with datafusion (literally the second page on the docs) so it should be clearer.

What changes are included in this PR?

Improvements on the Documentation:

  • the "Example Usage" case
  • the SELECT page of SQL

nanicpc avatar Sep 07 '22 10:09 nanicpc

Codecov Report

Merging #3385 (21487dd) into master (d16457a) will increase coverage by 0.00%. The diff coverage is n/a.

:exclamation: Current head 21487dd differs from pull request most recent head d39cbdd. Consider uploading reports for the commit d39cbdd to get more accurate results

@@           Coverage Diff           @@
##           master    #3385   +/-   ##
=======================================
  Coverage   85.58%   85.58%           
=======================================
  Files         296      296           
  Lines       54175    54187   +12     
=======================================
+ Hits        46364    46377   +13     
+ Misses       7811     7810    -1     
Impacted Files Coverage Δ
...fusion/optimizer/src/single_distinct_to_groupby.rs 98.33% <0.00%> (+0.11%) :arrow_up:
datafusion/expr/src/logical_plan/plan.rs 77.61% <0.00%> (+0.16%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

codecov-commenter avatar Sep 07 '22 12:09 codecov-commenter

Marking as draft to signify this PR has planned changes (and make it easier to find PRs that are in need of review). Please mark "ready for review" when it is next ready or if this change was a mistake.

alamb avatar Sep 12 '22 11:09 alamb

What do you think about changing the introductory example in the user guide @andygrove ? Given it is the first thing some people see I think there is a potential reason to keep it as simple as possible (and thus maybe it would be nice to leave it uncapitalized, and add add a link about the important subtlety of capitalization to somewhere later?

If you agree I will merge this PR and then make a follow on PR that tries to retain the simple initial example as well as the capitalized example

alamb avatar Oct 11 '22 10:10 alamb

What do you think about changing the introductory example in the user guide @andygrove ? Given it is the first thing some people see I think there is a potential reason to keep it as simple as possible (and thus maybe it would be nice to leave it uncapitalized, and add add a link about the important subtlety of capitalization to somewhere later?

If you agree I will merge this PR and then make a follow on PR that tries to retain the simple initial example as well as the capitalized example

That sounds great. Thanks @alamb

andygrove avatar Oct 12 '22 20:10 andygrove

Thanks again @nanicpc

alamb avatar Oct 12 '22 20:10 alamb

Benchmark runs are scheduled for baseline = d5c361b63c31b57d0052c501a94c1b1f7847b402 and contender = d5e6736dd286c0c0374bb0f65b88d3564155c976. d5e6736dd286c0c0374bb0f65b88d3564155c976 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. Conbench compare runs links: [Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ec2-t3-xlarge-us-east-2] ec2-t3-xlarge-us-east-2 [Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on test-mac-arm] test-mac-arm [Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ursa-i9-9960x] ursa-i9-9960x [Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ursa-thinkcentre-m75q] ursa-thinkcentre-m75q Buildkite builds: Supported benchmarks: ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True test-mac-arm: Supported benchmark langs: C++, Python, R ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

ursabot avatar Oct 12 '22 20:10 ursabot

Follow on PR in https://github.com/apache/arrow-datafusion/pull/3815

alamb avatar Oct 12 '22 20:10 alamb