
Creating a blank record causes an error with Postgres
Reported by Timothy Bennett (lanaer) | December 6th, 2007 @ 12:16 AM
irb(main):007:0> Node.create({})
DataObject::QueryError: Your query failed.
ERROR: syntax error at end of input
LINE 1: INSERT INTO "nodes"
^
QUERY: "INSERT INTO "nodes""
from /gentoo/usr/lib/ruby/gems/1.8/gems/do_postgres-0.2.2/lib/do_postgres.rb:186:in `execute_non_query'
from /gentoo/usr/lib/ruby/gems/1.8/gems/datamapper-0.2.4/lib/data_mapper/adapters/data_object_adapter.rb:209:in `save'
from /gentoo/usr/lib/ruby/gems/1.8/gems/datamapper-0.2.4/lib/data_mapper/adapters/data_object_adapter.rb:68:in `connection'
from /gentoo/usr/lib/ruby/gems/1.8/gems/datamapper-0.2.4/lib/data_mapper/support/connection_pool.rb:68:in `hold'
from /gentoo/usr/lib/ruby/gems/1.8/gems/datamapper-0.2.4/lib/data_mapper/adapters/data_object_adapter.rb:68:in `connection'
from /gentoo/usr/lib/ruby/gems/1.8/gems/datamapper-0.2.4/lib/data_mapper/adapters/data_object_adapter.rb:208:in `save'
from /gentoo/usr/lib/ruby/gems/1.8/gems/datamapper-0.2.4/lib/data_mapper/context.rb:54:in `save'
from /gentoo/usr/lib/ruby/gems/1.8/gems/datamapper-0.2.4/lib/data_mapper/support/active_record_impersonation.rb:12:in `save'
from /gentoo/usr/lib/ruby/gems/1.8/gems/datamapper-0.2.4/lib/data_mapper/support/active_record_impersonation.rb:71:in `create'
from (irb):7
Methinks it should be possible to create a record with no data yet besides the ID (although it’s generally pointless, sure).
I don’t know if this affects other adapters.
Comments and changes to this ticket
-
Jim December 6th, 2007 @ 01:39 PM
I have found the same for MySQL.
I think any empty Model will crash on new.
class N < Datamapper::Base end N.new #=> crashes
-
Sam Smoot December 28th, 2007 @ 10:52 PM
- Milestone cleared.
- State changed from new to open
I think most databases will simply fail trying to do this because, what fields do we include in the insert statement?
But... Jim's example shouldn't crash... that's just crazy-talk. I'll need to fix that.
Also, I should probably raise some sort of error if you ::new an object with no properties defined? Not sure about that one...
But need to give more feedback on the why at least since it's easy to identify the problem at runtime. Any ideas on what you guys would ideally like to see happen?
-
Jim December 29th, 2007 @ 09:54 AM
There is (in this case) an automatic field of id. Isn't that enuf for
a valid model?
Jim
-
Sam Smoot December 29th, 2007 @ 10:04 AM
There is an id, right, but serial keys are not part of the INSERT statement. It's my assumption most databases will do one of two things if you pass a serial key in an INSERT:
1. The query will fail, telling you you need to adjust a constraint or something to enable identity-insert.
2. The query will succeed, with the key set to whatever you passed. In this case NULL probably, which will probably in turn violate a NOT NULL constraint and blow up anyways.
So... AR wouldn't allow this I imagine, so I guess I'm curious to know why we would want to try to support creating a record for a table with only an id?
It might be possible to just:
INSERT INTO mytable
With no column list at all I suppose... In years of working with SQL, I don't think I've ever actually tried such a thing. :-D So color me ignorant on this one. ;-)
Anyways...
I guess what I'd like to see is a spec or description of some sort in support of a valid use-case for this. Otherwise it seems like we should just raise an error telling users there's no properties defined to help out the peeps new to DM. I'm especially in favor of that since it seems a not uncommon misunderstanding for newcomers.
So if we decide not to raise an error, and instead try to make these single-column serial-key only tables work, I want to make sure we've actually got a good use-case for it.
-
Timothy Bennett (lanaer) December 29th, 2007 @ 10:46 AM
For reference, AR can do this (by filling the SQL statement with the default values that the DB would be using anyway).
As for an actual use-case… I really can’t think of a solid one. Perhaps if you were setting up a relationship of some kind and needed an ID, but didn’t need it to be populated yet. Or perhaps to put a limit on how many of a certain object you have, and an “empty” one is simply not used yet.
Or maybe you have well-chosen default values for each field, and just need a record populated with those defaults.
Actually, that last one seems most plausible to me. Probably won’t come up frequently, but it seems a silly thing to not be able to just create a record with the default values unless you explicitly specify one of those defaults in creation.
Granted, certainly wouldn’t make it a high priority ;)
-
Jim December 29th, 2007 @ 10:50 AM
On Dec 29, 2007, at 10:04 AM, Lighthouse wrote:
// Add your reply here
I agree that time shouldn't be spent to make this work. Just to
improve the error message to make it clear that there are no
attributes defined for the model.
Jim
-
Sam Smoot December 29th, 2007 @ 11:03 AM
Timothy,
I agree about most of the use-cases, especially the last, except in this ticket, the assumption is you have no properties defined. So there are no useful defaults.
Basically, the use-cases you suggest are good, but they don't seem to apply to this ticket since the root of the problem here is that no attributes are defined.
So I guess with your guy's contributions, I'm going to go ahead and move forward with raising an error for this case. Something like raising a IncompleteModelDefinitionError or something.
Suggestions are welcome of course. I don't really like that name for the error, but can't think of anything better at the moment... :-)
-
Timothy Bennett (lanaer) December 29th, 2007 @ 11:20 AM
Woah, properties were defined on my model when I ran into this issue. Guess I should’ve explained that more clearly. Sorry about that.
My model:
class Node < DataMapper::Base property :title, :string, :nullable => false property :slug, :string, :nullable => false property :syndicate, :boolean property :published_at, :datetime property :parent_id, :integer, :default => nil, :nullable => true property :position, :integer, :default => nil, :nullable => true property :draft, :boolean, :default => false is_a_tree :order => "position" # … end
What happens with that model:
>> Node.create({})DataObject::QueryError: Your query failed. ERROR: syntax error at end of input LINE 1: INSERT INTO "nodes" ^ QUERY: "INSERT INTO "nodes""
-
Timothy Bennett (lanaer) December 29th, 2007 @ 11:24 AM
Blah, copy-paste from terminal messed up my newlines again and I didn’t fix all of them.
>> Node.create({}) DataObject::QueryError: Your query failed. ERROR: syntax error at end of input LINE 1: INSERT INTO "nodes" ^ QUERY: "INSERT INTO "nodes""
Lighthouse should have a “preview” feature since you can’t edit your updates ;)
-
Sam Smoot December 29th, 2007 @ 11:05 PM
Timothy,
Ooooh... My bad. :-)
Hmm... So... what do you suggest? When there are no dirty attributes we insert maybe the first nullable field we find?
Or maybe :default gets assigned on the Ruby-side in a before-create? That would handle this particular case nicely...
I guess I'm wondering if you have any ideas for what the generated query should look like, and how we determine how to get from here (no dirty attributes) to there (using columns X, Y and Z in the insert statement with ??? values (NULL? empty String?)).
-
Timothy Bennett (lanaer) January 1st, 2008 @ 06:09 PM
I’d probably just pick the first field in line, and explicitly include the default value or NULL in the insert statement.
-
Sam Smoot January 1st, 2008 @ 11:38 PM
Timothy: I don't know why, but that seems a little... distasteful to me. :-)
Not that I can think of anything better tho'... So ok, I'll commit to making that change, and if we think of something better, we can always change it later.
-
Timothy Bennett (lanaer) January 1st, 2008 @ 11:50 PM
Yeah, I wish I could think of something better as well. The ideal solution would be to just do a plain INSERT INTO tablename;, but sadly SQL doesn’t allow that, and I haven’t been able to find a way to do something similar.
The other two options are to either consistently explicitly provide default values in the INSERT statements (I believe AR does this), which as the advantage of consistency, but then your queries will be needlessly bloated; or to do as you are: in the case of an empty insert, find just one column to provide a default for.
There is a third option, though, now that I think more. But I don’t really like this one much either: to ask the database what id should be assigned next, and then explicitly give it.
-
Bernerd Schaefer January 6th, 2008 @ 10:39 AM
In MySql you can do: INSERT INTO users () VALUES ();
I haven't figured out what you can do in other databases, though...
-
Bernerd Schaefer January 6th, 2008 @ 11:13 AM
Postgres and sqlite3 both support: INSERT into users DEFAULT VALUES; Cool.
-
Timothy Bennett (lanaer) January 6th, 2008 @ 11:30 AM
Alright, Sam, looks like Bernerd is smarter than the two of us ;)
His solution seems best.
-
Bernerd Schaefer January 7th, 2008 @ 08:30 AM
In r687 you can now create an empty record. MySql uses "insert into table () values ()", while all others use "insert into table default values".
-
Bernerd Schaefer January 7th, 2008 @ 08:43 AM
- Assigned user set to Bernerd Schaefer
- State changed from open to resolved
I also went ahead and added #check_for_properties! which gets called at the beginning of #initialize in DM::Persistence. It raises an IncompleteModelDefinitionError if there are no properties defined.
Please Sign in or create a free account to add a new ticket.
With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
People watching this ticket
Referenced by
-
134 Malformed SQL in attribute-less save The specs pass when running against DataMapper trunk (r7...