dbcli / pgspecial

Python implementation of postgres meta commands (backslash commands)
BSD 3-Clause "New" or "Revised" License
74 stars 54 forks source link

Complete named query aggregation feature #113

Closed chagui closed 3 years ago

chagui commented 3 years ago

Description

This pull-request complete #106 with the possibility to compose named query positional arguments with the recently introduced arguments aggregation. The tests have been extended in order to reflect the desired behavior. Also includes a bug fix for named query detection, add a missing tox dependency and clean-up of an unused import.

Checklist

codecov-commenter commented 3 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@52f6e2e). Click here to learn what that means. The diff coverage is 65.86%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #113   +/-   ##
=========================================
  Coverage          ?   29.24%           
=========================================
  Files             ?        6           
  Lines             ?     2500           
  Branches          ?        0           
=========================================
  Hits              ?      731           
  Misses            ?     1769           
  Partials          ?        0           
Impacted Files Coverage Δ
pgspecial/help/commands.py 0.20% <ø> (ø)
pgspecial/main.py 38.18% <52.17%> (ø)
pgspecial/dbcommands.py 33.60% <66.73%> (ø)
pgspecial/iocommands.py 42.69% <68.05%> (ø)
pgspecial/namedqueries.py 92.00% <75.00%> (ø)
pgspecial/__init__.py 100.00% <100.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 52f6e2e...fc122c8. Read the comment docs.

lgtm-com[bot] commented 3 years ago

This pull request fixes 1 alert when merging fc122c8b88e54f5398f9de14c12dafdd8deb9121 into 52f6e2ed76321257e514c4e2221475f41c3d070a - view on LGTM.com

fixed alerts:

chagui commented 3 years ago

I preserved the independence between a placeholder substitution and its position in the named query. So the positional placeholders substitution takes place before the aggregation ones. I updated test query for positional and aggregation test to reflect better the behavior. One "edge case" remaining is that positional placeholders are being replaced as long as they are in increasing order. If there is a gap then everything after the gap will be swallowed by the aggregation placeholder, eventually causing an error. It is reflected in the test missing aggregation arguments with positional. If you feel like the later behavior needs to be adapted let me know.

On a side note I am preparing a PR for updating the docs to explain this ; it will be ready hopefully tomorrow.

chagui commented 3 years ago

Added docs.