DB2 support in radiant. Patch proposal

Hi.
I am experimenting IBM DB2 Express as a back-end for Radiant. Results
are encouraging so far. Some problems needed tweaks to the ibm_db
adapter for rails or to environment settings (see tracker on
http://rubyforge.org/projects/rubyibm/ for details). People at IBM have
been very responsive. Though more complicated to set up vis-a-vis mysql
or postgres, db2 has native support for xml, and this is the reason why
I am interested in using it in rails.
I propose the following patch to radiant/app/models/archive_finder.rb in
order to support db2 specific syntax for date fields:

def extract(part, field)
#puts ActiveRecord::Base.connection.adapter_name
case ActiveRecord::Base.connection.adapter_name
#start of patch
when /ibm_db/i
format = case part
when /year/i
“YEAR(#{field})”
when /month/i
“MONTH(#{field})”
when /day/i
“DAY(#{field})”
end
format
#end of patch
when /sqlite/i
format = case part
when /year/i
‘%Y’
when /month/i
‘%m’
when /day/i
‘%d’
end
“CAST(STRFTIME(‘#{format}’, #{field}) AS INTEGER)”
when /sqlserver/i
“DATEPART(#{part.upcase}, #{field})”
else
“EXTRACT(#{part.upcase} FROM #{field})”
end
end

Luca Erzegovesi

I am interested in using it in rails.
I propose the following patch to
radiant/app/models/archive_finder.rb in
order to support db2 specific syntax for date fields:

def extract(part, field)

Oh god… ewwwwwwww…

In the interests of sanity, lets pretend that radiant never had
a switch based on ActiveRecord::Base.connection.adapter_name.

http://dev.radiantcms.org/radiant/changeset/447

I’d propose that we dump the archive_finder completely - it’s
only used from the tags, and now that the code is sane, there’s
no need to keep it locked in a corner. Any objections?

Dan.

Daniel S. wrote:

In the interests of sanity, lets pretend that radiant never had
a switch based on ActiveRecord::Base.connection.adapter_name.

http://dev.radiantcms.org/radiant/changeset/447

Did this change preserve existing functionality?

I’d propose that we dump the archive_finder completely - it’s
only used from the tags, and now that the code is sane, there’s
no need to keep it locked in a corner. Any objections?

And replace it with what? I’m not sure what you are proposing.


John L.
http://wiseheartdesign.com

Luca,

In the future you can submit a ticket on the Trac with the prefix
[PATCH].

Sean

Daniel S. wrote:

In the interests of sanity, lets pretend that radiant never had
a switch based on ActiveRecord::Base.connection.adapter_name.

http://dev.radiantcms.org/radiant/changeset/447

Did this change preserve existing functionality?

Yes - all tests pass, and unless there’s a database that doesn’t
allow comparison on date fields or I’m reading the code wrong it
will do exactly the same thing.

I’d propose that we dump the archive_finder completely - it’s
only used from the tags, and now that the code is sane, there’s
no need to keep it locked in a corner. Any objections?

And replace it with what? I’m not sure what you are proposing.

Each of the methods in archive_finder are used in exactly one place -
the corresponding archive_xx_index_page tag. The code used to be quite
complicated, and the day finder depended on the year and month finder.
Now however, the code is quite simple and has no interdependencies,
so I think the code would be clearer just inlined into the tags.

For example, the archive:children tag in archive_day_index_page would
be:

tag “archive:children” do |tag|
if request_uri =~ %r{/(\d{4})/(\d{2})/(\d{2})/?$}
start = Time.local($1, $2, $3)
finish = start.tomorrow
tag.locals.children = parent.children.find(:all,
:conditions => [
‘published_at >= ? and published_at < ?’,
start,
finish,
]
)
tag.expand
end
end

(code is actually more compact, but I’ve formatted across many lines
because something is wrapping my lines wierdly).

Dan.

IIRC, that ‘extract’ method that was removed was designed to deal with
sqlite3. Also, the size of your range is only one day.

Sean

IIRC, that ‘extract’ method that was removed was designed to
deal with sqlite3.

Yes, but it was only necessary because the original code was doing the
search in a platform specific way by relying on functions to
transform a date into a string and perform a string comparison. The new
approach just does a comparison on the fields as
dates and checks that it lies within a range, so there’s nothing special
about it.

Also the new approach would allow you to put an index on the
published_at column to get that running zippy - most db’s don’t deal
well with functions and indexes. Not that that’s going to have any real
impact until you’ve got A LOT of pages, but it’s nice
anyway.

Also, the size of your range is only one day.

Yes, that’s because my example was for the day index page - the month
index would use Time#next_month and the year index
Time#next_year.

Dan.

Daniel S. wrote:

Each of the methods in archive_finder are used in exactly one place -
the corresponding archive_xx_index_page tag. The code used to be quite
complicated, and the day finder depended on the year and month finder.
Now however, the code is quite simple and has no interdependencies,
so I think the code would be clearer just inlined into the tags.

I’m all for clarity. Go for it.


John L.
http://wiseheartdesign.com