custom type obliterated by from_statement
Somehow the process_result_value is getting lost when from_statement is in use, see attached test case
Comments (8)
-
repo owner -
repo owner specifically, query.from_statement() means, "run this Core expression and get a result set back, then run that data into an entity, matching on name if no columns() are present" . Query does not run process_result_value, that's part of the Core result set. so your types have to be present in the core part of this.
-
reporter To play devils' advocate, when the name match occurs, shouldn't the matched columns'
process_result_value
be called?(As I only came across this exploring solutions for
#3933I've no further active usecase for this) -
repo owner the whole "name matching" thing is a hack in the first place, the newer concept of text() allowing direct column correspondence with the columns() method is better. that way you associate a full column w/ type up front.
process_result_value on the ORM entity can't be unconditionally called because the Core expression might already have types in it that have done this step. An intricate way of checking would be to test the type of each column for "NullType" and then run it, but overall this goes against all the abstraction layers in place by asking the ORM to reach deep inside the internals of Core and run the type processors.
short answer is that the expression passed into from_statement() should be linked to the columns one wishes to associate with directly, name matching is never the best way to go. it was a bad idea.
-
repo owner this does lead to another shortcoming in the API is that if you do have your Table that's against the view, how do we link those columns back to the original Table without using name matching. at least with aliased(v, Test.table, match_names=True) the name matching is between two Table objects and typing is not an issue, but this does suggest if we ever had a full blown "View" object, it might want to have the ability to relate its columns to other Table columns. There's ways to make this happen now but they require building custom objects, using a similar trick as _make_proxy() in https://bitbucket.org/zzzeek/sqlalchemy/wiki/UsageRecipes/Views .
-
reporter One note from an API user pov; the existence of TextClause.columns is undiscoverable from looking at the docs for sql.expression.text maybe columns should be added as a keyword argument to the sql.expression.text method?
-
repo owner all the documentation has this thing where there's the "lowercase" function and you can click through to the Text() object to see the API, but also the .columns() method is demonstrated in the examples at http://docs.sqlalchemy.org/en/latest/core/sqlelement.html?highlight=text#sqlalchemy.sql.expression.text (" The TextClause.bindparams() method is used to provide bound parameter detail, and TextClause.columns() method allows specification of return columns including names and types:") It's also referred to in the description of the "typemap" argument which is deprecated. I'm not so into adding everything as kw arguments these days since it's cleaner to use method chaining. It's also in the ORM tutorial as mentioned earlier.
-
repo owner - changed status to wontfix
ill call this "wontfix" because i will grant that the behavior is unintuitive, and in general the area of relating two selectables to each other is still underdeveloped from a front-facing API perspective, but I don't think any changes should be made to the API here since there is a strict boundary between a Core selectable and how a Query interprets the rows from it.
- Log in to comment
not a bug, the text() object has untyped columns by default and these must be given types, see http://docs.sqlalchemy.org/en/latest/core/sqlelement.html?highlight=text#sqlalchemy.sql.expression.TextClause.columns, simplest way is to re-map your columns directly as in: