pypika icon indicating copy to clipboard operation
pypika copied to clipboard

Cannot build query with field named "field"

Open thunderk opened this issue 4 years ago • 4 comments

Maybe it comes from my way of using Query, but here is a simple example to demonstrate:

from pypika.queries import Schema, Query

table=Schema("schema").table

print(Query.from_(table).select(table.notfield))
# outputs: SELECT "notfield" FROM "schema"."table"

print(Query.from_(table).select(table.field))
# outputs: SELECT <bound method Selectable.field of Table('table', schema='<pypika.queries.Schema object at 0x7fa69f56ff60>')> FROM "schema"."table"

thunderk avatar Aug 13 '21 12:08 thunderk

Giving more thought about it, it's logical, I should use Field("field", table=table) instead of the attribute shortcut.

But I think it's confusing to allow getattr access, but that it's working only if there isn't a method/attribute/property of the same name in the Table class.

thunderk avatar Aug 13 '21 12:08 thunderk

Yes, it's a bit confusing, but that provides us the convenience of writing mytable.any_column_name_you_want without having to define that Field explicitly

anhqle avatar Aug 21 '21 17:08 anhqle

I understand the convenience it provides, but I still think it's a risky implementation that can break users' code in the future.

If a user has code that uses a mytable.data field, and then pypika adds a data attribute (or method, or property) to the Table implementation, the user code would break on upgrade, because the attribute access doesn't go through __getattr__ anymore.

I'd suggest to have something like mytable.fields.any_column_name_you_want, where fields would be an object with only the getattr mechanism, and no other attributes, keeping the convenience but without the name clash risk. The current __getattr__ can even redirect to the new system for backward compatibility (maybe with a depreciation warning).

What do you think?

Edit: backquoting

thunderk avatar Sep 08 '21 15:09 thunderk

Your point about breaking users' code makes sense. I'm not a core dev though, so I can't opine on whether the convenience trade-off is worth it :)

anhqle avatar Oct 04 '21 20:10 anhqle