DAL improvements

[Niphlod]

Hi @all,
    finally I got some time to start coding what I call "support for funny names".
We discussed this - I think - briefly before, and I said that I would have committed some time to it.
Well, the moment is arrived. Purpose of the "greatest patch of them all" is to cleanly separate model from underlying backend structure, meaning that you can have a db.table.field that maps "transparently" to whatever$the heck.isthiscorrect,you're.insane in any "raw" interaction with the db itself.
Admittedly, I started just with table names, so ATM DAL passes all unittests mapping db.table.field to whatever whatever$the heck.isthiscorrect,you're.field
When I'll have this finished, I see instantly this "niceties":
 - working with legacy tables just got better
 - users needing to "obscurate" real table names can do that
 - users needing to observe some sort of naming convention for db tables (like, all tables starting with t_) can save lots of typing
 - general code shortings, and possibility to map better to "some object oriented quasi-ORM" paradigm, being able to do
    db.persons.name == 'a' and let it transparently map to public.t_webapp_persons.full_name
 - reserved_keywords can be used for either table names or field names
 - intra-db (for MSSQL-like backends) and/or intra-schema (Postgresql comes to mind) queries

This is achieved - as discussed before, autoquoting an unquoted string leaves out some of the previously mentioned queries, but can be very well added as Yet Another Additional Feature - passing the RAW QUOTED tablename string to the table definition.
Code example
db.define_table('easy_name',
     Field('name'),
     rname='"this is the easy_name table"')

Now...as said earlier all current tests in test_dal.py with just the "funny table name" support are passing without hiccups. Additionally, I'm replicating most of those tests adding the "funny name" alternative and seeking that they pass too.
Who fiddled previously with dal.py knows that its kinda of a nightmare, every corner you touch leads to side-issues ^_^
What I'm asking to you is: e.g. I faced a gnarly issue with select(left=) that I predicted would be a particular PITA ... do you foresee any complication with a particular piece of DAL code ?
Are you confident that if test_dal.py passes (and the one's I'm replicating just adding "rname=somethingstrange" to all table definitions) there will be further issues in the framework ?

[Massimo DiPierro]

I cannot answer yes or no but this is useful and I will test it when you submit the patch. :-)

--
-- mail from:GoogleGroups "web2py-developers" mailing list
make speech: web2py-d...@googlegroups.com
unsubscribe: web2py-develop...@googlegroups.com
details : http://groups.google.com/group/web2py-developers
the project: http://code.google.com/p/web2py/
official : http://www.web2py.com/
---
You received this message because you are subscribed to the Google Groups "web2py-developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to web2py-develop...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Niphlod]

PR sent. As commit message says, its highly experimental.
I'm looking to add rname also for fields.

@Alan: you may want to check if rname needs additional care with the db.as_json() / db.from_dict() etc.... that part I left untouched 'cause I though you'd be faster to adapt the code, if needed.

I think the "biggest" thing to resolve is to adopt a new format for the .table files, to be able to auto_import the models.
As of right now I don't think that any attribute for the table is taken care of, only fields....

In the process, we may look forward to "how to store indexes" (will be a list of "something" attached to the table instance), so let's be smart.

Tested on SQLite, MySQL and PostgreSQL thanks to travis-ci.
I couldn't have done this without travis and without unittest.
PostgreSQL may need additional care when adopting rname, because you'll probably need to specify a sequence_name for the table as the algo web2py uses by default is not omniscient (btw, will be needed only if rname is REEEEALLY "out of the box").
 
This has really been the first time I develop with TDD something for web2py: I wish life could be as easy as it's been this time. Not that it's been an easy ride...but at least one is confident that is not breaking anything (hopefully).

MySQL has been the usual PITA. When they say I hate it, they're right... ;-)

I'll test MSSQL in the weekend.

I guess the only "big" missing is Oracle, but I'm happy to leave it to someone willing to do it :-P

[Niphlod]

ok, tests on MSSQL with the latest PR pass without problems.

I'm giving a try with rname on Field()s too and **it seems** to be really less complicated than with tablenames. I'm throwing at it the full dal test-suite and it's giving me no issues at least on SQLite and Postgresql.

I don't know if we don't test all features in test_dal.py or if this was indeed an easy enhancement....I expected far more problems. Kinda make me sad that to have postponed this feature so long.... ^__^

Anyhow, stay tuned for the next release.

[Marin Pranjić]

I left you a note on PR:

https://github.com/niphlod/web2py/commit/c9b0c4547a26b6329c26235c6fd99d37a34b082d#commitcomment-4391389

so basically form now on you cannot do:

db.define_table('table1', Field('bla', 'reference table2')

db.define_table('table2')

Marin

[Niphlod]

ok. main point is..... until that patch it was possible but surely neither documented nor advised....

[Niphlod]

btw.... what happens for lazy_tables usually ?

does access to a table define also the referenced one ?

[Marin Pranjić]

True, but we may wish to keep old behavior.

I am facing this problem because I like to keep table definitions separated - one table per model.

Before this patch I didn't have to worry about order of execution (only db.py had to be first), and I didn't, so now my apps break.

It is not a big issu,. I just need to rename 2-3 models. But I wonder how many broken apps will this change cause.

Marin

[Niphlod]

@Marin: let's keep the discussion here, since its pretty long

wait a sec.
the moment you deploy your app with a fresh database table creation will fail badly even with the previous dal code.....

to summarize:
it's true that web2py never errored out with

db.define_table('b', Field('a', 'reference a'))
db.define_table('a', Field('c'))

but this can only happen if you had in your code

db.define_table('a', Field('c'))
db.define_table('b', Field('a', 'reference a'))

and then refactored it (in the wrong way)

The unordered piece of code on empty databases will ALWAYS fail (because web2py doesn't reorder table definitions), so they **should** be in the correct order if you want database portability (and/or start a new instance of that app pointing to a new database)

[Niphlod]

[Massimo DiPierro]

This is a problem.

In the original version of web2py tables had to be defined in order (as Simone suggests). Then many people made the case that users need to circular references:

db.define_table('b', Field('a', 'reference a'))

db.define_table('a', Field('b', 'reference b'))

This was explicitly supported. Therefore this also must be supported as particular case:

db.define_table('b', Field('a', 'reference a'))
db.define_table('a', Field('c'))

If there is anything in trunk that breaks it may have to be reverted.

What exactly causes the breaking?

Massimo

On Oct 22, 2013, at 7:23 AM, Niphlod wrote:

@Marin: let's keep the discussion here, since its pretty long

wait a sec.
the moment you deploy your app with a fresh database table creation will fail badly even with the previous dal code.....

to summarize:
it's true that web2py never errored out with

db.define_table('b', Field('a', 'reference a'))
db.define_table('a', Field('c'))

but is also true that that piece of code on empty databases will ALWAYS fail (because web2py doesn't reoder table definitions), so they need to be in the correct order...

db.define_table('a', Field('c'))
db.define_table('b', Field('a', 'reference a'))

[Marin Pranjić]

That's what I thought but actually it's not failing, at least not with SQLite.
Massimo has a good point. We may agree that what I do is wrong, but circular references also break.

@Massimo

https://github.com/niphlod/web2py/commit/c9b0c4547a26b6329c26235c6fd99d37a34b082d#commitcomment-4391389

It raises:

<type 'exceptions.AttributeError'>('DAL' object has no attribute 'table2')

[Massimo Di Pierro]

I understand why this breaks because the first reference is unaware of the rname of the referenced table. There is an easy solution, if the referenced table is undefined rname should default to the name of the referenced table. We can, for now, restrict out of order references to not have rnames.

Niphlod, what do you think?

Massimo

On Tuesday, 22 October 2013 07:37:13 UTC-5, Massimo Di Pierro wrote:

This is a problem.

In the original version of web2py tables had to be defined in order (as Simone suggests). Then many people made the case that users need to circular references:

db.define_table('b', Field('a', 'reference a'))

db.define_table('a', Field('b', 'reference b'))

This was explicitly supported. Therefore this also must be supported as particular case:

db.define_table('b', Field('a', 'reference a'))
db.define_table('a', Field('c'))

If there is anything in trunk that breaks it may have to be reverted.

What exactly causes the breaking?

Massimo

On Oct 22, 2013, at 7:23 AM, Niphlod wrote:

@Marin: let's keep the discussion here, since its pretty long

wait a sec.
the moment you deploy your app with a fresh database table creation will fail badly even with the previous dal code.....

to summarize:
it's true that web2py never errored out with

db.define_table('b', Field('a', 'reference a'))
db.define_table('a', Field('c'))

but is also true that that piece of code on empty databases will ALWAYS fail (because web2py doesn't reoder table definitions), so they need to be in the correct order...

db.define_table('a', Field('c'))
db.define_table('b', Field('a', 'reference a'))

--
-- mail from:GoogleGroups "web2py-developers" mailing list

make speech: web2py-developers@googlegroups.com
unsubscribe: web2py-developers+unsubscribe@googlegroups.com

details : http://groups.google.com/group/web2py-developers
the project: http://code.google.com/p/web2py/
official : http://www.web2py.com/
---
You received this message because you are subscribed to the Google Groups "web2py-developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to web2py-developers+unsubscribe@googlegroups.com.

[Massimo DiPierro]

>>> db = DAL()

>>> db.define_table('b', Field('a', 'reference a'))

Traceback (most recent call last):

  File "<console>", line 1, in <module>

  File "/Users/massimodipierro/Dropbox/web2py/gluon/dal.py", line 7970, in define_table

    table = self.lazy_define_table(tablename,*fields,**args)

  File "/Users/massimodipierro/Dropbox/web2py/gluon/dal.py", line 8007, in lazy_define_table

    polymodel=polymodel)

  File "/Users/massimodipierro/Dropbox/web2py/gluon/dal.py", line 867, in create_table

    real_referenced = db[referenced]._rname or db[referenced]

  File "/Users/massimodipierro/Dropbox/web2py/gluon/dal.py", line 8069, in __getitem__

    return self.__getattr__(str(key))

  File "/Users/massimodipierro/Dropbox/web2py/gluon/dal.py", line 8076, in __getattr__

    return ogetattr(self, key)

AttributeError: 'DAL' object has no attribute 'a'

This should not fail. This is is (was) allowed by web2py.

Massimo

[Anthony]

Did you mean that to be Field('a', 'reference b')?

[Massimo DiPierro]

No. I mean reference a (a table yet to be defined).

[Massimo DiPierro]

This is why we moved from this syntax:

   db.define_table('b', Field('a', db.a))

to this syntax:

   db.define_table('b', Field('a', 'reference a'))

because table "a" may not be defined yet.

Massimo

On Oct 22, 2013, at 8:34 AM, Anthony wrote:

[Anthony]

Oh, I thought you meant it failed after making the fix you mentioned (indicating a further problem).

[Niphlod]

ehm. there's a conceptual problem.
Leave alone SQLite that practically doesn't know about references....

take postgresql. You can't issue a create table statement that involves a field that references another table if that other table doesn't exist in the db.

So, the syntax

db.define_table('a', Field('reference b'))

actually can't create a table as defined.

How to manage that ?
Are we saying that we want to support bad refactorings?

[Niphlod]

an example is better than a thousand words.

NOT WORKING

CREATE TABLE auth_cas
(
  id serial NOT NULL,
  user_id integer,
  CONSTRAINT auth_cas_pkey PRIMARY KEY (id ),
  CONSTRAINT auth_cas_user_id_fkey FOREIGN KEY (user_id)
      REFERENCES auth_user (id) MATCH SIMPLE
      ON UPDATE NO ACTION ON DELETE CASCADE
);
CREATE TABLE auth_user
(
    id serial NOT NULL,
    CONSTRAINT auth_user_pkey PRIMARY KEY (id )
);

WORKING


CREATE TABLE auth_user
(
    id serial NOT NULL,
    CONSTRAINT auth_user_pkey PRIMARY KEY (id )
);
CREATE TABLE auth_cas
(
  id serial NOT NULL,
  user_id integer,
  CONSTRAINT auth_cas_pkey PRIMARY KEY (id ),
  CONSTRAINT auth_cas_user_id_fkey FOREIGN KEY (user_id)
      REFERENCES auth_user (id) MATCH SIMPLE
      ON UPDATE NO ACTION ON DELETE CASCADE
);

[Marin Pranjić]

I don't see a difference :(

[Niphlod]

sorry, I edited the post as soon as I pressed submit :D

[Niphlod]

we need to agree on something here.

Web2py never had the powers to create tables as

db.define_table('a', 'reference b')
db.define_table('b', 'reference a')

It's impossible at SQL level to do so with real constraints.
This is an excerpt from 2.6.3 (long before rname support was added)
>>> db2 = DAL('postgres://niphlod:niphlod@localhost/prova')
>>> db2.define_table('t_a', Field('f_b', 'reference t_b'))
        ProgrammingError: relation "t_b" does not exist

So, let's talk about something real...... that syntax was NEVER achievable in the first place.

Now we can talk about circumventing the issue, but don't blame rname: it just showed that SQLite does not behave as any other relational backend.

[Michele Comitini]

can the creation of constraints be postponed somehow?

2013/10/22 Niphlod <nip...@gmail.com>

--

[Marin Pranjić]

True, that would not work. But once tables are defined (with or without web2py DAL), circular references should work. That line in DAL breaks them.
You are saying that we can't support circular references because we can't trigger such create table statements. Well, we do support them, we just can't create them.

Actually, we *could* support it if we wanted to.
1. CREATE TABLE table1 ... (without FK)
2. <remember that FK is missing for table2>
3. CREATE TABLE table2
4. ALTER TABLE table1... (add FK)

[Niphlod]

yep. Again... we can talk about how to circumvent the issue, but the issue is there (and was there) in the first place.

[Massimo Di Pierro]

So far this worked as long as it was done in steps:

Step 1)

db.define_table('a')

db.define_table('b', 'reference a')

Step 2)

db.define_table('a', 'reference b')
db.define_table('b', 'reference a')

So people have working code which is broken by trunk. One way or another we cannot commit code that would break their apps.

Massimo

[Niphlod]

One thing here.
Agreed on the fact that out-of-order definitions can't lead to the creation of the table, we can see it handy just to enable the relation at model's level.

I'm +1 on treating them consistently, but it would be a huge pain to coordinate the code, because web2py doesn't know when all tables are defined (to reach that point where it should start to issue all the ALTER statements to create the FK, knowing that there are no other tables)
 
If instead we want them to work just to be able to define relationships at the model's level, then feel free to put the code in a try:except.
Another "beware of the dogs":
right now at least
db.define_table('nodes', Field('father_id', 'reference nodes'), rname='"a very complicated"')
(that in my POV is the only one web2py should support, self-references)
works perfectly.

[Marin Pranjić]

Massimo, I think your suggestion is fine:
https://groups.google.com/d/msg/web2py-developers/cU_ZpBAOXAU/QUlbjbDYCisJ

But since we raised this issue... we could make circular references work in 1 step if we postpone foreign keys (as described above). Why not?

Marin

[Niphlod]

PS: this just popped up
https://groups.google.com/d/msg/web2py/_YZ6A3OoF3g/FN1Sqgmqm_YJ
Yet another example on how fragile is working with SQLite when developing a model and letting web2py handle table creations out of order.

:-P

[Massimo DiPierro]

The question is not whether it is a good idea or not. I will agree it is a bad idea. Yet today people have code that does this:

db.define_table('a', Field('b', 'reference b'))

db.define_table('b', Field('a','reference a'))

and we are not going to break their code. How they got there is their business. They may have created tables outside web2py. They may have done it in steps.

The ability to reference tables that have not yet been defined has been introduced as a feature on request of the users.

Years ago a case was made for this and it was agreed upon. The rationale was that this may be a problem with the database but web2py should not get in the way and prevent it.

This must be fixed or we will have to revert the patch and I really do not want to do it.

Do you want to take a crack at fixing it, before I try and I mess up your code? ;-)

Massimo 

[Massimo DiPierro]

The question is not whether it is a good idea or not. I will agree it is a bad idea. Yet today people have code that does this:

db.define_table('a', Field('b', 'reference b'))

db.define_table('b', Field('a','reference a'))

and we are not going to break their code. How they got there is their business. They may have created tables outside web2py. They may have done it in steps.

The ability to reference tables that have not yet been defined has been introduced as a feature on request of the users.

Years ago a case was made for this and it was agreed upon. The rationale was that this may be a problem with the database but web2py should not get in the way and prevent it.

This must be fixed or we will have to revert the patch and I really do not want to do it.

Do you want to take a crack at fixing it, before I try and I mess up your code? ;-)

Massimo 

On Oct 22, 2013, at 4:40 PM, Niphlod wrote:

[Niphlod]

I'll try this evening, but it's not going to be any beautier than (pseudo-code)

if try-to-do-the-right-thing goes into exception:

      do-the-bad-thing

:P

[Massimo DiPierro]

Mind, I am not saying that the new rname feature has to work for out-of-order tables. That would not be possible/easy to accomplish.

[Niphlod]

I get it. 

BTW....how can out-of-order tables be tested with unittests?

Given that we can't redefine tables in the same thread, I'm having a hard time figuring out what the test should - eventually - look like.

[Richard Vézina]

+100 Simone rname feature

It is the missing piece to me to jump into SQLFORM.grid(). It could be one of the killer feature of web2py, because even big database companie that offer big software package don't handle right the table name. I mean there is always a moment where an "end user" (more advanced user, but not a developper or someone that should have to were about the backend names) will have to mess with the backend tables and fields name to create report or complex query... It is the case now with SQLFORM.grid(), that why I can't using it, because I don't want my users to have to figure it out or even more problematic guess which field is what.

And in context of Service Oriented Architecture - SOA, where someone want to create a service that allow excel to get data directly, it would help too I think... You don't have to do the rename on purpose for each services or create you own mechanism to handle it.

Hope what I wrote makes sens.

Thanks Simone for this, and hope there will be a proper way to don't break backward operability

Richard

[Massimo DiPierro]

actually we can db.define_table(...,redefine=True)

[Niphlod]

Il giorno mercoledì 23 ottobre 2013 17:33:34 UTC+2, Massimo Di Pierro ha scritto:

actually we can db.define_table(...,redefine=True)

will this trigger a migration based on the previous datamodel stored into the .table files ? 

[Massimo DiPierro]

yes. It should.

[Niphlod]

I can't seem to find a way to unittest out-of-order redefinitions. But the PR I just sent (https://github.com/web2py/web2py/pull/278) fixes the issue when testing manually.
If someone is willing to make a test for it, it will save for any future trouble relating to this

Basically we need to have a way to unittest:

db.define_table('table1', Field('name'))
db.define_table('table2', Field('name'), Field('table1_id', 'reference table1'))

to create the tables and then simulate another request (i.e. db._tables is empty)

db.define_table('table1', Field('name'), Field('table2_id', 'reference table2'))
#that will trigger a migration on table1, adding the constraint to table2
db.define_table('table2', Field('name'), Field('table1_id', 'reference table1'))
#that will do nothing

[Niphlod]

https://github.com/web2py/web2py/pull/285
adds support for rname in Fields too.

@all: please, throw at it every model you can think of and, most importantly, add tests for it!
@all2: someone tested it on Oracle ?
@all3: we need to agree on a new "format" for the .table files that carry around additional attributes for the tables....soon support for indexes will be added and we'd need to take care of it too.
@alan: didn't touch the db.as_dict(), etc. this time again.... can you make sure that everything works out in that end ?

@massimo: while I was there I noticed that a PR slipped in without any support for python < 2.7 ( https://github.com/web2py/web2py/pull/282 ).
My PR fixes it too, but keep an eye for new features merged into trunk.
It's true that we want to remain "agile" when pushing new feaures (hence its not strictly required to add tests for every new feature) but this could have been slipped in without anyone noticing it ^__^

[Alan Etkin]

> @alan: didn't touch the db.as_dict(), etc. this time again.... can you make sure that everything works out in that end ?

A bit late I think. The dal.py test obviously passes, but with a mongo connection this fails:

>>> dbasdict = db.as_dict(flat=True, sanitize=False)
>>> otherdb = DAL(**dbasdict)

Traceback (most recent call last):
  File "<console>", line 1, in <module>

  File "/home/spametki/web2py/web2py-git/gluon/dal.py", line 7805, in __init__
    tables=tables)
  File "/home/spametki/web2py/web2py-git/gluon/dal.py", line 7813, in import_table_definitions
    pattern = pjoin(path,self._uri_hash+'_*.table')
  File "/usr/lib/python2.7/posixpath.py", line 68, in join
    elif path == '' or path.endswith('/'):
AttributeError: 'NoneType' object has no attribute 'endswith'

Anyway, I'm not sure this actually worked before.

[Massimo DiPierro]

Was this fixed?

[Alan Etkin]

Was this fixed?

I think so:
https://code.google.com/p/web2py/issues/detail?id=1795#c2

Thanks