Patch review request: add index length support (#1852)

This patch add support for index length in MySQL adapter. Define a index length is a common practice for avoiding large indexes data, and improving performance.

You can now define a length for you indexes:

add_index(:accounts, :name, :name => ‘by_name’, :limit => 10) generates CREATE INDEX by_name ON accounts(name(10))

add_index(:accounts, [:name, :surname], :name => ‘by_name_surname’, :limit => 10) generates CREATE INDEX by_name_surname ON accounts(name(10), surname(10))

add_index(:accounts, [:name, :surname], :name => ‘by_name_surname’, :limit => {:name => 10, :surname => 20}) generates CREATE INDEX by_name_surname ON accounts(name(10), surname(20))

http://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/1852-patch-to-add-index-length-support

Thanks.

Emili Parreño wrote:

This patch add support for index length in MySQL adapter. Define a index length is a common practice for avoiding large indexes data, and improving performance.

See this ticket with more tests and schema dumper support:

http://rails.lighthouseapp.com/projects/8994/tickets/734-add-support-for-index-length-in-mysql-adapter-and-schema-dumper#ticket-734-2

Jonathan

Fine! but I would replace this:

add_index(:accounts, [:name, :surname], :name => 'by_name_surname',
:limit => {:name => 10, :surname => 20})

by:

add_index(:accounts, {:name => 10, :surname => 20}, :name =>
'by_name_surname'), which also cover the first situation (with a
simple index)

add_index(:accounts, :name => 10, :name => 'by_name')

With this, I don't think that the :limit optionis really needed.

Atenciosamente,

Carlos Henrique Júnior
Milk-it Software House
http://www.milk-it.net
+55 31 3227 1009
+55 31 8763 5606

Hash doesn't preserve order, which is critical when declaring indices.

Hum, you're right... so we can't do it until 1.9 becomes a default
ruby env (which makes every hash ordered).

Atenciosamente,

Carlos Henrique Júnior
Milk-it Software House
http://www.milk-it.net
+55 31 3227 1009
+55 31 8763 5606

Hello list,

It came to my surprise when I read the 2.3 RC1 release notes, that engines
have been added back into core.
This has apparently happened in the dark so that hardly anyone noticed,
although it's great to have them back.

When trying to use my existing "engine" (which I used with the desert gem in
2.2) I encountered a couple of issues however, that made my initial smile
vanish very soon.

First the routes in the engine's config/routes.rb file seem to override all
existing ones which can hardly be intentional. Has really no one tested this
or am I doing it the completely wrong way?

I got it to "work" by adding the routes to the regular routes.rb file and
removing config/routes.rb from the plugin that that has immediately raised
another issue.. Namespaced controllers inside the engine don't seem to be
recognized or even loaded, although a namespace imho makes sense especially
in that scenario.

Before filing a report on Lighthouse and looking into the code, I wanted to
ensure it hasn't been my fault (especially with the routes) so if there's
anything I should know, please shoot.

Thanks to all contributors for their great work that went into 2.3 however!

Cheers

Pascal

Hello list,

It came to my surprise when I read the 2.3 RC1 release notes, that engines
have been added back into core.
This has apparently happened in the dark so that hardly anyone noticed,
although it's great to have them back.

When trying to use my existing "engine" (which I used with the desert gem in
2.2) I encountered a couple of issues however, that made my initial smile
vanish very soon.

FYI, I haven't looked at edge recently but desert is generally
functional against edge. The specs run against all releases + edge,
so it's easy to determine. We (Pivotal Labs) would love to fold that
functionality into rails, but will be supporting it against
(minimally) all official versions of rails until that happens. Let us
know if there are bugs.

pt.

Hi,

Sounds good, I remember having some problems when I tried to use desert with
edge a few days ago but that may have been related to another problem.

I will give it a try soon..
Talking about desert bugs, I never got migrations working, there's always
been a weird error message that doesn't easily come to my mind again but I
will let you know as soon as I remember.

Thanks for the reply and thanks for desert!

Pascal

Turns out that I've been wrong on the second complaint where namespaced
controllers didn't work.

I think the problem was that CVS (yeah, we're seriously still using this as
work) screwed up my plugin directory and removed all files.

The issue with the routes being overridden still exists, though.

Best,

Pascal

The issue with the routes being overridden still exists, though.

OK, can you open a new lighthouse ticket with your example and assign
it to DHH? The engine functionality in 2.3 isn't identical to the
engines-plugin but it should let you achieve most of the same stuff.

Hi Koz,

From what I understood from a blog post on the engines-plugin site, routes
should be working, though.
It really wouldn't make any sense to just override all existing ones, that's
why I was wondering whether nobody has encountered this issue before.

I think we can make a deal that I update the documentation (especially the
guides are outdated and don't apply to the new engines stuff anymore) if
someone else fixes the implementation. :wink:

I will add a Lighthouse ticket tomorrow (need to get some sleep now).

Cheers,

Pascal

I updated de patch with more test, schema support, and I fixed a failure in SQLite tests.

Thanks.

http://rails.lighthouseapp.com/projects/8994/tickets/1852