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

select / query unification #5159

Closed
zzzeek opened this issue Feb 17, 2020 · 5 comments
Closed

select / query unification #5159

zzzeek opened this issue Feb 17, 2020 · 5 comments
Labels
alchemy 2 goes along with the 2.0 milestone to aid in searching big job a major change that requires a lot of knowledge and work
Milestone

Comments

@zzzeek
Copy link
Member

zzzeek commented Feb 17, 2020

building on the "move compile context to compile" at https://gerrit.sqlalchemy.org/#/c/sqlalchemy/sqlalchemy/+/1599/

Query needs to be a Select, and select() and query() constructs need to share as much code as possible as far as how they collect state:

  1. both will accumulate _criterion, _order_by, _group_by, _from_obj as tuples that build up

  2. for select compile, dont use clauselist at all. Accessors like select._wherecllause, select._order_by_clause will create a ClaustList on the fly for backwards compatibility. but compiler will use:

text = self.render_tuple_with_separator(select._order_by, ",")
text = self.render_tuple_with_separator(select._criterion, "AND")

that way there is no expense or need to create ClauseList; the tuples already consist of arguments that have been vetted at construction time.

  1. Query has to subclass Select or some similar construct

  2. _compile_context does all the work in the above gerrit and is somehow triggerde from compilation; i think hardwiring a _process() method on the object can do this

  3. select() may or may not do the same thing

from the above, we hope to have:

  1. Query and select() are super cheap to build

  2. compilation of select() remains super cheap too, don't even bother with ClauseList

  3. all of Query compile_context moves to act within the compile phase

from there we can do #4639 have both be cached on cache key

@zzzeek zzzeek added alchemy 2 goes along with the 2.0 milestone to aid in searching big job a major change that requires a lot of knowledge and work labels Feb 17, 2020
@zzzeek zzzeek added this to 1.4 -> 2.0 transitional deliverables backlog in sqlalchemy 2 via automation Feb 17, 2020
@zzzeek zzzeek added this to the 1.4 milestone Feb 17, 2020
@zzzeek zzzeek changed the title select / query select / query unification Feb 17, 2020
@zzzeek
Copy link
Member Author

zzzeek commented Mar 6, 2020

we can remove:

  1. from_self(). the thing this accomplishes can be done more explicitly with aliased()

  2. select_from_entity(). same

this combined with #4705 we should be able to remove all adaption from Query / QueryContext.

at the moment I might want to build out the ORM context for select() directly first and try to improve upon the work done in I19e05b3424b07114cce6c439b05198ac47f7ac10 as much as possible, building from scratch with all of the old assumptions removed.

@zzzeek
Copy link
Member Author

zzzeek commented Mar 6, 2020

at the moment, 2.0 only needs to have select(), just once. there does not need to be any ORM specific front-facing construct, meaning, there is no Query. This will take the presssure off of 1.4 for Query to be a different kind of object, we can leave it mostly or entirely the way it is, if that turns out to be better.

when you have a select(), the entities if they are ORM entities will have an annotation "context_plugin", which points to a CompileState class that should be used when this object is compiled. Only the first plugin in the list of columns will be used. the coercions system should pull out this plugin right at the outset.

This means if I make a select() against an ORM entity and the ORM entity has joined loaders in it, they'll be in the SQL if for example I say str(statement).

there should be ways to affect the plugins, suppose we have this:

stmt = select(User).join(User.addresses)

that's an ORM join so we need the ORM plugin in order to compile the above construct. Maybe if they want the above without eager loaders we say:

select(User).join(User.addresses).plugin_options(enable_eagerloads=False)

@zzzeek zzzeek moved this from 1.4 -> 2.0 transitional deliverables backlog to 1.4->2.0 transitional deliverables in progress in sqlalchemy 2 Apr 7, 2020
@sqla-tester
Copy link
Collaborator

Mike Bayer referenced this issue:

Unify Query and select() , move all processing to compile phase https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/1599

sqlalchemy-bot pushed a commit that referenced this issue May 25, 2020
Convert Query to do virtually all compile state computation
in the _compile_context() phase, and organize it all
such that a plain select() construct may also be used as the
source of information in order to generate ORM query state.
This makes it such that Query is not needed except for
its additional methods like from_self() which are all to
be deprecated.

The construction of ORM state will occur beyond the
caching boundary when the new execution model is integrated.

future select() gains a working join() and filter_by() method.
as we continue to rebase and merge each commit in the steps,
callcounts continue to bump around.  will have to look at
the final result when it's all in.

References: #5159
References: #4705
References: #4639
References: #4871
References: #5010

Change-Id: I19e05b3424b07114cce6c439b05198ac47f7ac10
@zzzeek
Copy link
Member Author

zzzeek commented Jun 1, 2020

this is done in dce8c7a .

@zzzeek zzzeek closed this as completed Jun 1, 2020
sqlalchemy 2 automation moved this from 1.4->2.0 transitional deliverables in progress to 1.4 -> 2.0 transitional deliverables done Jun 1, 2020
@sqla-tester
Copy link
Collaborator

Mike Bayer referenced this issue:

Propose a single select() construct https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/2058

sqlalchemy-bot pushed a commit that referenced this issue Jul 8, 2020
Several weeks of using the future_select() construct
has led to the proposal there be just one select() construct
again which features the new join() method, and otherwise accepts
both the 1.x and 2.x argument styles.   This would make
migration simpler and reduce confusion.

However, confusion may be increased by the fact that select().join()
is different  Current thinking is we may be better off
with a few hard behavioral changes to old and relatively unknown APIs
rather than trying to play both sides within two extremely similar
but subtly different APIs.  At the moment, the .join() thing seems
to be the only behavioral change that occurs without the user
taking any explicit steps.   Session.execute() will still
behave the old way as we are adding a future flag.

This change also adds the "future" flag to Session() and
session.execute(), so that interpretation of the incoming statement,
as well as that the new style result is returned, does not
occur for existing applications unless they add the use
of this flag.

The change in general is moving the "removed in 2.0" system
further along where we want the test suite to fully pass
even if the SQLALCHEMY_WARN_20 flag is set.

Get many tests to pass when SQLALCHEMY_WARN_20 is set; this
should be ongoing after this patch merges.

Improve the RemovedIn20 warning; these are all deprecated
"since" 1.4, so ensure that's what the messages read.
Make sure the inforamtion link is on all warnings.
Add deprecation warnings for parameters present and
add warnings to all FromClause.select() types of methods.

Fixes: #5379
Fixes: #5284
Change-Id: I765a0b912b3dcd0e995426427d8bb7997cbffd51
References: #5159
openstack-mirroring pushed a commit to openstack/tacker that referenced this issue Jul 12, 2021
This patch fixes a database migration for SQLAlchemy 1.4.

In tacker repository following issues are encounter:

1. Replace deprecated SQLAlchemy "with_lockmode" method.
   The method was deprecated in SQLAlchemy 0.9 [1][2]
   and finally removed in version 1.4.
   This patch replaces "with_lockmode" with new method
   "with_for_update", which has no end-user impact.

2. The clause column IN, in the new version it requires parameters
   passed to operator either be a list of literal value
   or a tuple, or an empty list [3].

3. Observe an argument error in 'update' query.
   Error: sqlalchemy.exc.ArgumentError: subject table for an INSERT,
   UPDATE or DELETE expected, got Column('id', Uuid(length=36),
   table=<vnf_software_images>, primary_key=True, nullable=False,
   default=ColumnDefault(<function generate_uuid at 0x7fb5be371d30>))

4. The INSERT statement requires values to add in table as a list,
   tuple or dictionary [4].
   Error: sqlalchemy.exc.ArgumentError: mapping or sequence expected
          for parameters
   This patch creates a dictionary of fields to be insert in
   VnfLcmOpOccs table.

5. Query fails on applying filters.
   When applying query to list vnf package or vnf instance using
   apply_filters() method exported from sqlalchemy_filters,
   it failed.
   AttributeError: 'Query' object has no attribute '_join_entities'

   In SQLAlchemy 1.4 unification of "query" and "select" construct
   took place [5][6].
   The issue observe in tacker is an open bug of SQLAlchemy [7].
   This patch drops "sqlalchemy_filters" and add customize method
   to apply filters in query.

Note: This patch address sqlalchemy errors, SAWarnings will be resolve
in future patches.

[1] https://docs.sqlalchemy.org/en/13/changelog/migration_09.html?highlight=with_lockmode#new-for-update-support-on-select-query
[2] https://docs.sqlalchemy.org/en/13/orm/query.html#sqlalchemy.orm.query.Query.with_lockmode
[3] https://docs.sqlalchemy.org/en/14/core/sqlelement.html#sqlalchemy.sql.expression.ColumnElement.in_
[4] https://docs.sqlalchemy.org/en/14/core/dml.html#dml-class-documentation-constructors
[5] sqlalchemy/sqlalchemy#5159
[6] https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/1599/
[7] juliotrigo/sqlalchemy-filters#61

Co-Authored-By: Ayumu Ueha <ueha.ayumu@fujitsu.com>

Change-Id: Ifd1ae2753c639f22fc1afa020222416fe79469ef
openstack-mirroring pushed a commit to openstack/openstack that referenced this issue Jul 12, 2021
* Update tacker from branch 'master'
  to a98cd4eaa9f2c1669f0a1d1c748d6d0028edd1f4
  - Merge "Fix migration for SQLAlchemy 1.4"
  - Fix migration for SQLAlchemy 1.4
    
    This patch fixes a database migration for SQLAlchemy 1.4.
    
    In tacker repository following issues are encounter:
    
    1. Replace deprecated SQLAlchemy "with_lockmode" method.
       The method was deprecated in SQLAlchemy 0.9 [1][2]
       and finally removed in version 1.4.
       This patch replaces "with_lockmode" with new method
       "with_for_update", which has no end-user impact.
    
    2. The clause column IN, in the new version it requires parameters
       passed to operator either be a list of literal value
       or a tuple, or an empty list [3].
    
    3. Observe an argument error in 'update' query.
       Error: sqlalchemy.exc.ArgumentError: subject table for an INSERT,
       UPDATE or DELETE expected, got Column('id', Uuid(length=36),
       table=<vnf_software_images>, primary_key=True, nullable=False,
       default=ColumnDefault(<function generate_uuid at 0x7fb5be371d30>))
    
    4. The INSERT statement requires values to add in table as a list,
       tuple or dictionary [4].
       Error: sqlalchemy.exc.ArgumentError: mapping or sequence expected
              for parameters
       This patch creates a dictionary of fields to be insert in
       VnfLcmOpOccs table.
    
    5. Query fails on applying filters.
       When applying query to list vnf package or vnf instance using
       apply_filters() method exported from sqlalchemy_filters,
       it failed.
       AttributeError: 'Query' object has no attribute '_join_entities'
    
       In SQLAlchemy 1.4 unification of "query" and "select" construct
       took place [5][6].
       The issue observe in tacker is an open bug of SQLAlchemy [7].
       This patch drops "sqlalchemy_filters" and add customize method
       to apply filters in query.
    
    Note: This patch address sqlalchemy errors, SAWarnings will be resolve
    in future patches.
    
    [1] https://docs.sqlalchemy.org/en/13/changelog/migration_09.html?highlight=with_lockmode#new-for-update-support-on-select-query
    [2] https://docs.sqlalchemy.org/en/13/orm/query.html#sqlalchemy.orm.query.Query.with_lockmode
    [3] https://docs.sqlalchemy.org/en/14/core/sqlelement.html#sqlalchemy.sql.expression.ColumnElement.in_
    [4] https://docs.sqlalchemy.org/en/14/core/dml.html#dml-class-documentation-constructors
    [5] sqlalchemy/sqlalchemy#5159
    [6] https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/1599/
    [7] juliotrigo/sqlalchemy-filters#61
    
    Co-Authored-By: Ayumu Ueha <ueha.ayumu@fujitsu.com>
    
    Change-Id: Ifd1ae2753c639f22fc1afa020222416fe79469ef
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alchemy 2 goes along with the 2.0 milestone to aid in searching big job a major change that requires a lot of knowledge and work
Projects
No open projects
sqlalchemy 2
  
1.4 -&gt; 2.0 transitional deliverabl...
Development

No branches or pull requests

2 participants