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

Fix unmatched param bug #162

Merged
merged 6 commits into from Jul 30, 2023
Merged

Fix unmatched param bug #162

merged 6 commits into from Jul 30, 2023

Conversation

bowiec
Copy link
Contributor

@bowiec bowiec commented Apr 21, 2023

Hi! I ran into a bug when using pydapper on my project when parameters are specified, but not used in the SQL query. In that case, it would throw a missing key exception. This might seem like a contrived situation, but it is an issue when using query_multiple where not all queries have parameters specified (which is what happened to me).

I also noticed the codecov package version in use is no longer available and was causing the install to fail so I updated it.

@zschumacher
Copy link
Owner

codecov actually deleted their python uploader from pypi, I'll put in a separate PR to fix that in CI

@zschumacher
Copy link
Owner

can you add an example to the PR of what the code would look like that isn't working? Specifically the query multiple version you mentioned?

@bowiec
Copy link
Contributor Author

bowiec commented Apr 21, 2023

Sure, here's a minimum example:

param = {
    "val": 1,
}

sql = (
    "select 1",
    "select ?val?"
)

val1, val2 = commands.query_multiple(sql, models=(dict, dict), param=param)

This results in this exception:

KeyError: "Key '' can not be accessed on {'val': 1} or does not exist"
obj = {'val': 1}, key = ''

    def safe_getattr(obj: Any, key: str) -> Any:
        try:
            if isinstance(obj, dict):
>               return obj[key]
E               KeyError: ''

Here's a more realistic example as well (closer to what I was doing):

param = {
    "id": 1,
}

sql = (
    "select * from table",
    "select * from another_table where id = ?id?"
)

val1, val2 = commands.query_multiple(sql, models=(dict, dict), param=param)

@zschumacher
Copy link
Owner

hey man - sorry I'm taking so long getting back to this, been a crazy few months for me. Lets try to get this merged this week.

@codecov
Copy link

codecov bot commented Jul 30, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (b14079c) 100.00% compared to head (6234185) 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #162   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           20        20           
  Lines          569       569           
  Branches        98        98           
=========================================
  Hits           569       569           
Flag Coverage Δ
3.10-bigquery 71.17% <100.00%> (ø)
3.10-core 87.34% <100.00%> (ø)
3.10-mssql 72.05% <100.00%> (ø)
3.10-mysql 71.17% <100.00%> (ø)
3.10-oracle 72.05% <100.00%> (ø)
3.10-postgresql 85.94% <100.00%> (ø)
3.10-sqlite 70.82% <100.00%> (ø)
3.11-bigquery 71.17% <100.00%> (ø)
3.11-core 87.34% <100.00%> (ø)
3.11-mssql 72.05% <100.00%> (ø)
3.11-mysql 71.17% <100.00%> (ø)
3.11-oracle 72.05% <100.00%> (ø)
3.11-postgresql 85.94% <100.00%> (ø)
3.11-sqlite 70.82% <100.00%> (ø)
3.8-bigquery 71.25% <100.00%> (ø)
3.8-core 87.83% <100.00%> (ø)
3.8-mssql 72.13% <100.00%> (ø)
3.8-mysql 71.25% <100.00%> (ø)
3.8-oracle 72.13% <100.00%> (ø)
3.8-postgresql 86.06% <100.00%> (ø)
3.8-sqlite 70.89% <100.00%> (ø)
3.9-bigquery 71.25% <100.00%> (ø)
3.9-core 87.83% <100.00%> (ø)
3.9-mssql 72.13% <100.00%> (ø)
3.9-mysql 71.25% <100.00%> (ø)
3.9-oracle 72.13% <100.00%> (ø)
3.9-postgresql 86.06% <100.00%> (ø)
3.9-sqlite 70.89% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
pydapper/commands.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@zschumacher zschumacher merged commit 182bed6 into zschumacher:main Jul 30, 2023
31 checks passed
@bowiec
Copy link
Contributor Author

bowiec commented Jul 31, 2023

No worries, thanks for reviewing!

@bowiec bowiec deleted the fix-missing-param-bug branch July 31, 2023 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants