On Wednesday, September 4, 2013 11:49:39 AM UTC-7, GregD wrote:
> Jeremy-
>
> Updated gist:
> https://gist.github.com/6240781.git
>
> Down to 2 failing tests. Not sure how to fix these 2.
>
First, let me say that that is very impressive. Only 7 pending tests is
very good, some of the adapters that ship with Sequel have more pending
tests than that.
>
>
> Failures:
>
> 1) RcteTree Plugin should not populate :children associations for final
> level when loading descendants to a given level
> Failure/Error: nodes[0].associations[:children].map{|c1|
> c1.associations[:children]}.should == [nil, nil]
> expected: [nil, nil]
> got: [[], []] (using ==)
> Diff:
> @@ -1,2 +1,2 @@
> -[nil, nil]
> +[[], []]
> # ./spec/integration/plugin_test.rb:858:in `block (2 levels) in <top
> (required)>'
>
> I don't even know where to begin to figure out what is going on with this.
> Very complicated recursion sql is generated and looks valid (works in
> dbisql). Something to do with the eager method, maybe? Although all the
> other tests with eager are working.
>
You might want to add some debugging code to the `if level` block near line
297 of the plugin. It should be setting no_cache to true for the
grandchildren, but if for some reason the dataset is returning a level
value that is not equal to 1, it will be false, so the associations hash in
the grandchildren will be initialized to [] in line 307. You could try
changing `obj.values.delete(la)` to `obj.values.delete(la).to_i` on line
297 and see if that fixes it.
> 2) Database foreign key parsing should parse foreign key information
> into an array of hashes
> Failure/Error: @db.alter_table(:b){drop_foreign_key :e}
> Sequel::DatabaseError:
> Sequel::SqlAnywhere::SQLAnywhereException: Foreign key name 'A' not
> found
> # ./lib/sequel/database/misc.rb:402:in `raise_error'
> # ./lib/sequel/adapters/sqlanywhere.rb:97:in `block in execute_dui'
> # ./lib/sequel/database/connecting.rb:229:in `block in synchronize'
> # ./lib/sequel/connection_pool/threaded.rb:104:in `hold'
> # ./lib/sequel/database/connecting.rb:229:in `synchronize'
> # ./lib/sequel/adapters/sqlanywhere.rb:91:in `execute_dui'
> # ./lib/sequel/database/query.rb:43:in `execute_ddl'
> # ./lib/sequel/database/schema_methods.rb:377:in `block in
> apply_alter_table'
> # ./lib/sequel/database/schema_methods.rb:377:in `each'
> # ./lib/sequel/database/schema_methods.rb:377:in `apply_alter_table'
> # ./lib/sequel/database/schema_methods.rb:382:in
> `apply_alter_table_generator'
> # ./lib/sequel/extensions/constraint_validations.rb:283:in
> `apply_alter_table_generator'
> # ./lib/sequel/database/schema_methods.rb:78:in `alter_table'
> # ./spec/integration/schema_test.rb:227:in `block (2 levels) in <top
> (required)>'
>
> This one is 2 fold:
> 1) SqlAnywhere will not allow you to add or drop a column or a foreign key
> if if violates a uniqueness constraint. So, before adding/dropping I query
> the schema to see if I need to do something i.e. alter the column to not be
> null if it is going to be a foreign key in another table. So, I made
> the appropriate alter_table_sql to do so to get these tests to work. In
> doing so, I cause the above to happen because this statement:
>
> @db.alter_table(:b){drop_foreign_key :e}
>
> generates 2 identical ALTER TABLE statements in the operations list of:
>
> ALTER TABLE B DROP FOREIGN KEY A
> ALTER TABLE B DROP FOREIGN KEY A
>
> Both are valid. The problem is the second will fail because of the first
> one removed the fk. I can add uniq to the return value of Sequel method
> alter_table_sql_list and it will work, but don't like that. If I remove
> the code that is generating the first ALTER TABLE, then I get a different
> test that fails which is dropping of a column that is used as a fk. Code
> that is generating the first ALTER TABLE:
>
> when :drop_column
> sqls = []
> f_keys = foreign_key_list(table)
> constraint_names = f_keys.map{|h| h[:name] if
> h[:columns].include?(op[:name])}
> constraint_names.each do |constraint|
> sqls << alter_table_sql(table, {:op => :drop_constraint,
> :type => :foreign_key, :name => constraint}) <---------- right here.
> end
> sqls << "ALTER TABLE #{quote_schema_table(table)} DROP
> #{column_definition_sql(op)}"
>
I can think of two ways to handle this:
1) In the code that generates the first DROP FOREIGN KEY, find a way to
determine if it would meet the conditions such that the second DROP FOREIGN
KEY would be used, and don't add the first DROP FOREIGN KEY in that case.
2) Always have the first DROP FOREIGN KEY used, but dup and modify the op
to add a flag that the second DROP FOREIGN KEY code would check and not add
the DROP FOREIGN KEY in that case.
Of the two options, 1) might be easier. If someone calls drop_foreign_key
with a single symbol, you know you have to drop the column, and if your
drop column code already uses DROP FOREIGN KEY in that case, you probably
don't need to add it.
>
> 2) I had to comment out this part of the test because I can not figure out
> what I need to do to prevent the constraint violation that it is causing.
> I think I have to add a uniqueness index for the composite key and I still
> can't figure out the ALTER TABLE SQL for that and have been trying the
> SqlAnyhere Sybase Central client program. Hopefully, I can figure that one
> out here soon.
>
> ~line 220 of schema_test.rb
> #@db.alter_table(:a){add_index [:d, :c], :unique=>true}
> #@db.alter_table(:b){add_foreign_key [:f, :e], :a, :key=>[:d, :c]}
> #@pr[:b, [[:e], :a, [:pk, :c]], [[:f], :a, [:c]], [[:f], :a, [:d]],
> [[:f, :e], :a, [:d, :c]]]
>
> #@db.alter_table(:b){drop_foreign_key [:f, :e]}
> @pr[:b, [[:e], :a, [:pk, :c]], [[:f], :a, [:c]], [[:f], :a, [:d]]]
>
If that add_index is failing, I'm not sure why, unless unique composite
indexes must be specified differently on SQLAnywhere.
> The pending ones are with reasons:
> Pending:
> Sequel::Database should provide ability to check connections for validity
> # Not yet working on sqlanywhere
> # ./spec/integration/spec_helper.rb:75
>
> It looks like the sqlanywhere native gem acts just like the sybase client
> software dbisql and sybase central where if you disconnect and then issue
> an SQL statement it auto reconnects you to your last connection sending
> back the results.
>
Yuck. Is there no way to turn that off? That's a pretty horrible idea.
Think starting a transaction and having a disconnect happen midway
through. With transparent auto reconnection, Sequel thinks you are inside
a transaction, but SQLAnwhere is outside a transaction. That's not a good
situation to be in.
>
> Simple dataset operations with nasty table names should work correctly
> # Not yet working on sqlanywhere
> # ./spec/integration/spec_helper.rb:75
>
Expected.
> Sequel::Database should correctly escape multiple backslash strings
> # Not yet working on sqlanywhere
> # ./spec/integration/spec_helper.rb:75
>
Unless there is a good reason for this failing, I would look at it more
carefully. This sounds like it could lead to SQL injection. I'm just
guessing that from the spec description, as I can't find this spec
description in the current integration tests.
> Sequel::Database should properly escape identifiers
> # Not yet working on sqlanywhere
> # ./spec/integration/spec_helper.rb:75
>
Expected.
> Sequel::Dataset DSL support should work with bitwise shift operators
> # Not yet working on sqlanywhere
> # ./spec/integration/spec_helper.rb:75
>
Is there no way to emulate bitwise shifts on SQLAnywhere? Usually they can
be emulated with simple multiple or divide of power(2, arg).
> Bound Argument Types should handle blob type with embedded zeros
> # Not yet working on sqlanywhere
> # ./spec/integration/spec_helper.rb:75
>
Is there a way to hex-encode the blobs to allow this?
> Supported types should support generic file type
> # Not yet working on sqlanywhere
> # ./spec/integration/spec_helper.rb:75
>
Probably related to above issue.
> All of these have to do with escaping characters and the native
> sqlanywhere gem use of StringValueCStr and there is no other method to call
> that does not use that on the sql string that is passed in. I split the
> escape backslash strings into 2 tests. On that worked for sqlanywhere and
> the ones that did not putting them into a cspecify. I called it escape
> multiple backslash strings.
>
Ah, that's why I couldn't find it. Anyway, you might want to take another
look and convince yourself 100% that SQL injection is not possible in that
case.
> I can set up a JDBC sqlanyhere adapter. That should not take too long to
> do and see if these can pass.
>
That would be great.
I would definitely like to get this merged into Sequel when you have
finished. Ideally I would have a local SQLAnywhere installation in a VM
that I could use for testing (to make sure future Sequel versions don't
break SQLAnywhere support). If that's something you could walk me through,
I would appreciate it. However, even if there are things preventing me
from testing it locally, I still think it would be beneficial to merge it.
Thank you very much for your continued determination in getting the
sqlanywhere adapter in shape.
Thanks,
Jeremy
--
You received this message because you are subscribed to the Google Groups
"sequel-talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sequel-talk.
For more options, visit https://groups.google.com/groups/opt_out.
