Hi Ken, On 10/17/2014 01:09 PM, Ken Winter wrote: > On Thursday, October 16, 2014 12:13:20 PM UTC-4, Carl Meyer wrote: > > I don't see any way this is feasible, if you want it to cover raw SQL > executed through ``cursor.execute``. Are you planning to parse the SQL > for every raw statement, > > /Ken: Yes./ > > figure out if its an INSERT or UPDATE, > > /Ken: Yes./ > > and then > figure out which row(s) it might affect? > > Ken: No; at this point my code inserts an element that assigns the user > id to "last_updated_by" into the SET clause of an UPDATE query or the > VALUES clause of an INSERT query, then hands the modified query back to > the connector, which passes it on to the DB. > > At this point you're well into > re-implementing chunks of PostgreSQL in your app code. > > > Ken: No, just a modest bit of parsing and tweaking of SQL queries. My > code doesn't have to execute the queries in any way.
I admire your intrepidity! But on a project I'm responsible for, I'd never let this pass code review. SQL is a complex language; are you using a battle-tested SQL parser that you are confident handles every possible syntax variation correctly? Or are you hand-rolling a regex-based "parser" that will work correctly for the simplest queries, but break when it encounters anything you didn't anticipate? To name a few examples selected randomly from a brief perusal of the Postgres docs: - Does your code handle an INSERT with DEFAULT VALUES but no VALUES clause? - Does your code handle an UPDATE that touches multiple tables using a FROM clause? - Does your code handle writable CTEs? You may say "I just won't use any of those SQL features"; at that point you don't have a solution that is safe to hide from your developers, and you may as well . > Ken: So to implement this in Django, one would have to find the Django > counterpart of ZPsycopgDA.db.py and insert this code in the same way. Yes, you'll have to do this either by patching Django, or monkeypatching it at runtime. I think you'll be happier in the long run if you avoid doing either of those things. You'll also have to install a "current user ID" threadlocal for this to work, which introduces another way in which this solution won't really be transparent to your developers - they'll have to always make sure they install a threadlocal user ID before doing anything with the database. And if they install the wrong one, rather than being a clear local bugfix (as in the case of passing the wrong user to model.save()), it's a much more mysterious action-at-a-distance bug to track down. And, lastly, this solution still isn't really "universal" because it won't help if someone connects to the database directly instead of through your app. > Ken: Whaddya think? I think that you can almost certainly get this working for simple cases, and it might even be a fun exercise, but if it were my production project that I needed to maintain for the next several years, I wouldn't touch this solution with a ten-foot pole. Carl -- You received this message because you are subscribed to the Google Groups "Django users" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-users+unsubscr...@googlegroups.com. To post to this group, send email to django-users@googlegroups.com. Visit this group at http://groups.google.com/group/django-users. To view this discussion on the web visit https://groups.google.com/d/msgid/django-users/54417535.3000800%40oddbird.net. For more options, visit https://groups.google.com/d/optout.
