[Dbix-class] $rs->create() with no column values bombs on Oracle

[Lasse Makholm]

On Mon, Sep 15, 2014 at 4:35 PM, Peter Rabbitson <rabbit+dbic at rabbit.us>
wrote:

> On 09/15/2014 04:02 PM, Lasse Makholm wrote:
>
>>
>>
>> On Fri, Sep 12, 2014 at 6:25 AM, Peter Rabbitson <rabbit+dbic at rabbit.us
>> <mailto:rabbit+dbic at rabbit.us>> wrote:
>>
>>     On 09/11/2014 11:39 PM, Lasse Makholm wrote:
>>
>>
>>
>>         On Thu, Sep 11, 2014 at 7:58 PM, Peter Rabbitson
>>         <rabbit+dbic at rabbit.us <mailto:rabbit%2Bdbic at rabbit.us>
>>         <mailto:rabbit+dbic at rabbit.us
>>
>>         <mailto:rabbit%2Bdbic at rabbit.us>>__> wrote:
>>
>>              On 09/11/2014 05:48 PM, Lasse Makholm wrote:
>>
>>                  Hi,
>>
>>                  We're porting a DBIx::Class application from MySQL to
>>         Oracle,
>>                  containing
>>                  some insert statements with no column specifications by
>>         way of
>>                  $rs->create({}). This doesn't seem to work on Oracle.
>>
>>                  DBIx::Class::SQLMaker generates "INSERT INTO table
>>         DEFAULT VALUES
>>                  RETURNING id INTO ?" which, as far as I can figure is
>>         not valid
>>                  syntax
>>                  on Oracle. Neither the is the other common variant for
>> such
>>                  statements;
>>                  "INSERT INTO table () VALUES ()".
>>
>>
>>              This is clearly a bug that we've never tested for
>>         previously. Can
>>              you please confirm that the following works on your Oracle
>>         RDBMS
>>              (yes, the first ()pair is gone):
>>
>>              INSERT INTO $tablename VALUES(DEFAULT);
>>
>>
>>         That only works for a table with only one column. For more,
>>         you'd need
>>         to repeat DEFAULT for each column. Otherwise you get: "SQL Error:
>>         ORA-00947: not enough values".
>>
>>
>>     I see.
>>
>>
>>         I wonder how/if an explicit DEFAULT interacts with the typical
>>         scenario
>>         of having a before insert trigger pulling IDs from a sequence to
>>         implement auto increment columns...
>>
>>         I guess, in a sense, that question boils down whether there's any
>>         difference between:
>>
>>         INSERT INTO t (a, b) VALUES (DEFAULT, 42)
>>
>>         and:
>>
>>         INSERT INTO t (b) VALUES (42)
>>
>>         In any case, I don't know the answer but I suspect we have some
>>         resident
>>         Oracle experts who do... I'll discuss it with them and report
>>         back...
>>
>>
>>     Yes please do. What we are looking for basically is the most
>>     "universal" statement we could come up with. Putting it in is just a
>>     SMOP after that.
>>
>>
>> So, to the best of my knowledge and that of those I've talked to, there
>> is no way insert a row in Oracle without listing at least one column.
>>
>> That leaves picking an arbitrary list of columns and supplying the
>> DEFAULT keyword as value.
>>
>> I guess that leaves the question of how many and which columns to
>> specify in the insert statement. Choosing one arbitrary column seems
>> weird but including all columns seems overly redundant/wasteful. I'm not
>> sure which solution is less ugly...
>>
>
> In this case we will be using the first column as declared in add_columns
> on the result source (they are stored positionally).
>
> This will have to wait until after the current release (see next email),
> as the thing never worked in the first place. If you have the tuits to
> write a proto implementation and chuck it either to the ML or in a github
> PR - please do!


First stab:
https://github.com/dbsrgits/dbix-class/pull/61

I initially thought I could fix it in SQLMaker but quickly realised it
doesn't know about result sources and thus can't know which column to add
to the statement.

Instead, overriding insert() in DBIx::Class::Storage::DBI::Oracle::Generic
seems to work nicely.

I also tweaked t/60core.t to insert a row with no columns..

$source->create({}) would, ideally, be tested on all storage types, I
guess...?

Also, which branch is preferred for submitting new pull requests against?
Is it documented somewhere?

/L

Cheers
>
> _______________________________________________
> List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class
> IRC: irc.perl.org#dbix-class
> SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/
> Searchable Archive: http://www.grokbase.com/group/
> dbix-class at lists.scsys.co.uk
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.scsys.co.uk/pipermail/dbix-class/attachments/20140929/60bfba0f/attachment.htm>