Raise exception if as_dict=True and column(s) have no names #160

Merged
merged 2 commits into from Jan 9, 2014

Projects

None yet

3 participants

@msabramo
Member
msabramo commented Jan 9, 2014

This is an alternative solution to the issue in #157, suggested by @rsyring.

@rsyring proposed that we modify pymssql to raise an exception when the user uses as_dict=True and then does a query which returns output columns that are not named; the current behavior of omitting those columns is very confusing -- e.g.:

SELECT MAX(x)
FROM (VALUES (1), (2), (3)) AS foo(x)

Note that we don't provide a name for the MAX(x) column above.

cursor = conn.cursor(as_dict=True)
cursor.execute("SELECT MAX(x) FROM (VALUES (1), (2), (3)) AS foo(x)")
print(cursor.fetchall())
# Outputs:
# [{}]
# Whoa, what happened to MAX(x)?!?!

Currently, we don't add the column to the result row if it has no name. This confuses people. See this mailing list thread for an example.

I have a more detailed example at: https://gist.github.com/msabramo/8319097

So the new behavior with this change is:

>>> cursor.execute("SELECT MAX(x) FROM (VALUES (1), (2), (3)) AS foo(x)")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "pymssql.pyx", line 426, in pymssql.Cursor.execute (pymssql.c:5828)
    raise ColumnsWithoutNamesError(columns_without_names)
pymssql.ColumnsWithoutNamesError: Specified as_dict=True and there are columns with no names: [0]

and it works nicely even when there are multiple columns without names:

>>> cursor = conn.cursor(as_dict=True)
>>> cursor.execute("SELECT MAX(x), MIN(x) AS [MIN(x)], AVG(x) FROM (VALUES (1), (2), (3)) AS foo(x)")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "pymssql.pyx", line 426, in pymssql.Cursor.execute (pymssql.c:5828)
    raise ColumnsWithoutNamesError(columns_without_names)
pymssql.ColumnsWithoutNamesError: Specified as_dict=True and there are columns with no names: [0, 2]
@msabramo
Member
msabramo commented Jan 9, 2014

Cc: @rsyring, @ramiro, @damoxc

Let me know what you think.

@damoxc
Contributor
damoxc commented Jan 9, 2014

This seems sane to me, I can't really think of a situation where you would be using as_dict=True and have unnamed columns, I imagine it would just result in a KeyError down the line when they attempt to access the value.

@rsyring
Member
rsyring commented Jan 9, 2014

Yep, I like this route too.

@msabramo msabramo merged commit 495a174 into master Jan 9, 2014

1 check passed

default The Travis CI build passed
Details
@msabramo msabramo deleted the as_dict_No_column_name_raise_exception2 branch Jan 9, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment