<div dir="auto"><div><br><div class="gmail_extra"><br><div class="gmail_quote">On Jul 25, 2017 3:38 PM, "Octave J. Orgeron" <<a href="mailto:octave.orgeron@oracle.com">octave.orgeron@oracle.com</a>> wrote:<br type="attribution"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Michael,<br>
<br>
I understand that you want to abstract this completely away inside of oslo.db. However, the reality is that making column changes based purely on the size and type of that column, without understanding what that column is being used for is extremely dangerous. You could end up clobbering a column that needs a specific length for a value, </blockquote></div></div></div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto">Nowhere in my example is the current length truncated.   Also, if two distinct lengths truly must be maintained we add a field "minimum_length".</div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto">prevent</div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> an index from working, etc. </blockquote></div></div></div><div dir="auto"><br></div><div dir="auto">That's what the indexable flag would achieve.  </div><div dir="auto"><br></div><div dir="auto">It</div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> wouldn't make sense to just do global changes on a column based on the size.</blockquote></div></div></div><div dir="auto"><br></div><div dir="auto">This seems to be what your patches are doing, however.</div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> There are far more tables that fit in both InnoDB and NDB already than those that don't. As I've stated many times before, the columns that I make changes to are evaluated to understand:<br>
<br>
1. What populates it?<br>
2. Who consumes it?<br>
3. What are the possible values and required lengths?<br>
4. What is the impact of changing the size or type?<br>
5. Evaluated against the other columns in the table, which one makes the most sense to adjust?<br>
<br>
I don't see a way of automating that and making it maintainable without a lot more overhead in code and people. </blockquote></div></div></div><div dir="auto"><br></div><div dir="auto">My proposal is intended to *reduce* the great verbosity in the current patches I see and remove the burden of every project having to be aware of "ndb" every time a column is added.</div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto">If</div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> we really want to remove the complexity here, why don't we just change the sizes and types on these handful of table columns so that they fit within both InnoDB and NDB? </blockquote></div></div></div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto">Because that requires new migrations which are a great risk and inconvenience to projects.  </div><div dir="auto"><br></div><div dir="auto">That</div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> way we don't need these functions and the tables are exactly the same? That would only leave us with the createtable, savepoint/rollback, etc. stuff to address which is already taken care of in the ndb module in oslo.db? Then we just fix the foreign key stuff as I've been doing, since it has zero impact on InnoDB deployments and if anything ensures things are consistent. That would then leave us to really focus on fixing migrations to use oslo.db and pass the correct flags, which is a more lengthy process than the rest of this.<br>
<br>
I don't see the point in trying to make this stuff anymore complicated.</blockquote></div></div></div><div dir="auto"><br></div><div dir="auto">The proposal is to make it simpler than it is right now.</div><div dir="auto"><br></div><div dir="auto">Run though every column change youve proposed and show me which ones don't fit into my proposed ruleset.   I will add additional declarative flags to ensure those use cases are covered. </div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><font color="#888888"><br>
<br>
Octave</font><div class="elided-text"><br>
<br>
On 7/25/2017 12:20 PM, Michael Bayer wrote:<br>
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="elided-text">
On Mon, Jul 24, 2017 at 5:41 PM, Michael Bayer <<a href="mailto:mbayer@redhat.com" target="_blank">mbayer@redhat.com</a>> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
oslo_db.sqlalchemy.String(255, ndb_type=TINYTEXT) -> VARCHAR(255) for most<br>
dbs, TINYTEXT for ndb<br>
oslo_db.sqlalchemy.String(4096<wbr>, ndb_type=TEXT) -> VARCHAR(4096) for most<br>
dbs, TEXT for ndb<br>
oslo_db.sqlalchemy.String(255, ndb_size=64) -> VARCHAR(255) on most dbs,<br>
VARCHAR(64) on ndb<br>
<br>
This way, we can override the String with TINYTEXT or TEXT or change the<br>
size for ndb.<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
oslo_db.sqlalchemy.String(255)<wbr>     -> VARCHAR(255) on most dbs,<br>
TINYTEXT() on ndb<br>
oslo_db.sqlalchemy.String(255, ndb_size=64)     -> VARCHAR(255) on<br>
most dbs, VARCHAR(64) on ndb<br>
oslo_db.sqlalchemy.String(50)     -> VARCHAR(50) on all dbs<br>
oslo_db.sqlalchemy.String(64)     -> VARCHAR(64) on all dbs<br>
oslo_db.sqlalchemy.String(80)     -> VARCHAR(64) on most dbs, TINYTEXT()<br>
on ndb<br>
oslo_db.sqlalchemy.String(80, ndb_size=55)     -> VARCHAR(64) on most<br>
dbs, VARCHAR(55) on ndb<br>
<br>
don't worry about implementation, can the above declaration -><br>
datatype mapping work ?<br>
<br>
<br>
</blockquote>
In my patch for Neutron, you'll see a lot of the AutoStringText() calls to<br>
replace exceptionally long String columns (4096, 8192, and larger).<br>
</blockquote>
MySQL supports large VARCHAR now, OK.   yeah this could be<br>
String(8192, ndb_type=TEXT) as well.<br>
</blockquote>
OK, no, sorry each time I think of this I keep seeing the verbosity of<br>
imports etc. in the code, because if we had:<br>
<br>
String(80, ndb_type=TEXT)<br>
<br>
then we have to import both String and TEXT, and then what if there's<br>
ndb.TEXT, the code is still making an ndb-specific decision, etc.<br>
<br>
I still see that this can be mostly automated from a simple ruleset<br>
based on the size:<br>
<br>
length <= 64 :    VARCHAR(length) on all backends<br>
length > 64, length <= 255:   VARCHAR(length) for most backends,<br>
TINYTEXT for ndb<br>
length > 4096:  VARCHAR(length) for most backends, TEXT for ndb<br>
<br>
the one case that seems outside of this is:<br>
<br>
String(255)  where they have an index or key on the VARCHAR, and in<br>
fact they only need < 64 characters to be indexed.  In that case you<br>
don't want to use TINYTEXT, right?   So one exception:<br>
<br>
oslo_db.sqlalchemy.types.Strin<wbr>g(255, indexable=True)<br>
<br>
e.g. a declarative hint to the oslo_db backend to not use a LOB type.<br>
<br>
then we just need oslo_db.sqlalchemy.types.Strin<wbr>g, and virtually<br>
nothing except the import has to change, and a few keywords.<br>
<br>
What we're trying to do in oslo_db is as much as possible state the<br>
intent of a structure or datatype declaratively, and leave as much of<br>
the implementation up to oslo_db itself.<br>
<br></div><div class="quoted-text">
______________________________<wbr>______________________________<wbr>______________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.op<wbr>enstack.org?subject:unsubscrib<wbr>e</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi<wbr>-bin/mailman/listinfo/openstac<wbr>k-dev</a><br>
</div></blockquote><div class="elided-text">
<br>
<br>
<br>
______________________________<wbr>______________________________<wbr>______________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.op<wbr>enstack.org?subject:unsubscrib<wbr>e</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi<wbr>-bin/mailman/listinfo/openstac<wbr>k-dev</a><br>
</div></blockquote></div><br></div></div></div>