Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Allow Proper Functionality For Tables Without Auto-Increment PK #17

Merged
merged 1 commit into from

2 participants

Rob Goretsky Chris Parker
Rob Goretsky

I have been using the AR JDBC Teradata adapter to load data into previously existing tables in my database. In some cases, these tables do not have a standard Auto-Incremented "ID" column. For example, we could have a "Posts" table that has a "post_id" primary key that is set manually by my script.

This adapter had been mostly working for me in these cases, but I found that I was hitting an Exception when loading data that contained a string that had more than three periods in it.

I dug into the code, and found what looks to be a bug - on line 140 of lib/arjdbc/teradata/adapter.rb, we are calling last_insert_id and passing it the complete SQL statement. This looks to be incorrect, as the last_insert_id method actually expects a table name. I found that using the _table_name_from_insert(sql) method resolved this and made my spec now properly pass.

After making this change I ran all of the existing specs and they all continue to pass.

Rob Goretsky Create a new test case wherein we have a table with no auto-increment…
…ed primary key. Fix bug in adapter code that looks up PK based on the SQL when it should be passing the table name
f950d17
Chris Parker mrcsparker merged commit 0be793c into from
Rob Goretsky

Thanks for merging this in! Will you be releasing an updated gem to rubygems at some point with this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 18, 2013
  1. Create a new test case wherein we have a table with no auto-increment…

    Rob Goretsky authored
    …ed primary key. Fix bug in adapter code that looks up PK based on the SQL when it should be passing the table name
This page is out of date. Refresh to see the latest.
2  lib/arjdbc/teradata/adapter.rb
View
@@ -137,7 +137,7 @@ def _execute(sql, name = nil)
end if self.class.lowercase_schema_reflection
result
elsif self.class.insert?(sql)
- (@connection.execute_insert(sql) or last_insert_id(sql)).to_i
+ (@connection.execute_insert(sql) or last_insert_id(_table_name_from_insert(sql))).to_i
else
@connection.execute_update(sql)
end
18 spec/models/no_autoincrement_pk.rb
View
@@ -0,0 +1,18 @@
+class CreateNoAutoincrementPks < ActiveRecord::Migration
+ def self.up
+ create_table :no_autoincrement_pks, :id=>false do |t|
+ t.integer :post_id
+ t.string :title
+ t.timestamps
+ end
+ end
+
+ def self.down
+ drop_table :no_autoincrement_pks
+ end
+
+end
+
+class NoAutoincrementPk < ActiveRecord::Base
+ self.primary_key = 'post_id'
+end
17 spec/no_autoincrement_pk_spec.rb
View
@@ -0,0 +1,17 @@
+require 'spec_helper'
+require 'models/no_autoincrement_pk'
+
+describe 'NoAutoincrementPkSpec' do
+ it 'should be able to create a record and manually set the post_id' do
+ CreateNoAutoincrementPks.up
+ obj = NoAutoincrementPk.new
+ obj.post_id = 12345
+ obj.title = 'Sample title with 3 or more periods ...'
+ obj.save.should be_true
+ obj.reload
+ obj.id.should eq(12345)
+ found = NoAutoincrementPk.find(12345)
+ found.id.should eq(12345)
+ CreateNoAutoincrementPks.down
+ end
+end
Something went wrong with that request. Please try again.