On 10/09/2011 10:43 PM, Rudy Lippan wrote:
> On Saturday, October 08, 2011 at 09:39:50 PM, Mark Stosberg wrote:
> 
> 
>> My problem with Noah's patch is that it didn't find the new test that I
>> added to the ticket.
>>
>> https://rt.cpan.org/Ticket/Attacahment/973506/506656/
>>
>> So, either my test is wrong, or Noah's patch is incomplete. Could you
>> help confirm which is the case?
>>
> 
> $SQL = q{INSERT INTO dbd_pg_test (id,testarray2) VALUES };
>                                      ^^^^^^^^^^
> Should that be "testarray"?

Rudy,

Thanks for the reply. You correctly found a bug in my test case. The
bug is that "testarray2" is a column of type "int", while my value was
text. By contrast The column named "testarray" is of type "varchar", so
my quoting test  works as correctly.

Good news: Noah's patch then passes this new test that I added as well
as the original test.

Given this peer review and additional test coverage, and I
enthustiastically, recommend that Noah's patch be merged.

I think we will likely apply the patch locally and put it into
production, as the official upgrade path for DBD::Pg is currently broken
in this regard.

As a follow-up, I recommend the following change in the test suite. It
currently uses these column names:

  testarray  text[][],
  testarray2 int[],
  testarray3 bool[],

As we just observed, the poor name choices for these columns contributed
to a bug in a test update. I advocate a find/replace to the test suite
to give the test columns more descriptive names, which will make the
test suite easier to browser and correctly comprehend for new contributors:

  testarray_text  text[][],
  testarray_int   int[],
  testarray_bool  bool[],

Thanks,

   Mark


Reply via email to