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

rubyonrails-core
1

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.

2

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

3

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

4

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

5

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

6

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

7

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.

8

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

9

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

10

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.

11

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

12

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