New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump the max table size to 103 #1265

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
7 participants
@kochmaxence

kochmaxence commented Oct 20, 2017

feature gate 'insane-tables'

Used "insane-tables" instead of "really-huge-tables" to express the fact that it is likely a bad idea.

see #359 (comment)

Bump the max table size to 103
feature gate 'insane-tables'
@killercup

Woah, how long does it take to compile diesel with that feature? I think there were some recent improvements in rustc to speed this up, but I'd guess it still takes are least a few minutes in debug mode.

@@ -45,6 +45,7 @@ unstable = []
lint = ["clippy"]
large-tables = []
huge-tables = ["large-tables"]
insane-tables = ["huge-tables"]

This comment has been minimized.

@killercup

killercup Oct 22, 2017

Member

Can we go with "humongous-tables"? It fits better as the X in "large, huge, X"

@killercup

killercup Oct 22, 2017

Member

Can we go with "humongous-tables"? It fits better as the X in "large, huge, X"

@sgrif

This comment has been minimized.

Show comment
Hide comment
@sgrif

sgrif Oct 22, 2017

Member

I'm also curious what the compile times are with this feature on. I think we can probably just update huge-tables to this number instead of adding a new feature

Member

sgrif commented Oct 22, 2017

I'm also curious what the compile times are with this feature on. I think we can probably just update huge-tables to this number instead of adding a new feature

@kochmaxence

This comment has been minimized.

Show comment
Hide comment
@kochmaxence

kochmaxence Oct 23, 2017

It takes me about 15 minutes to compile diesel with the feature.

Fifteen. Minutes.

image

I can cope with "humongous-tables", instead of "insane-tables", "probably-a-bad-idea-tables", "intern-tables", "unreadable-tables", "anti-compile-tables" & others.

Unfortunately I have a table with 80 columns.

kochmaxence commented Oct 23, 2017

It takes me about 15 minutes to compile diesel with the feature.

Fifteen. Minutes.

image

I can cope with "humongous-tables", instead of "insane-tables", "probably-a-bad-idea-tables", "intern-tables", "unreadable-tables", "anti-compile-tables" & others.

Unfortunately I have a table with 80 columns.

@Eijebong

This comment has been minimized.

Show comment
Hide comment
@Eijebong

Eijebong Oct 23, 2017

Member

Why are you putting 103 if your table only has 80 ?

Member

Eijebong commented Oct 23, 2017

Why are you putting 103 if your table only has 80 ?

@theduke

This comment has been minimized.

Show comment
Hide comment
@theduke

theduke Oct 23, 2017

Contributor

15 minutes is a lot better then when I tried to implement the same thing.
It took around an hour for me with 128 columns, if I remember correctly.

It's still way too long to be sensible though, imo.

Contributor

theduke commented Oct 23, 2017

15 minutes is a lot better then when I tried to implement the same thing.
It took around an hour for me with 128 columns, if I remember correctly.

It's still way too long to be sensible though, imo.

@leodasvacas

This comment has been minimized.

Show comment
Hide comment
@leodasvacas

leodasvacas Nov 5, 2017

We should not use "insane" in a name because that may be perceived as ableist.

leodasvacas commented Nov 5, 2017

We should not use "insane" in a name because that may be perceived as ableist.

@nsrsr

This comment has been minimized.

Show comment
Hide comment
@nsrsr

nsrsr Dec 30, 2017

no, we should not. Please let's all work together on software instead of polluting it with so called "social issues" and politics of any kind. Please spare my area of work from left-wing ideas. Thanks.

nsrsr commented Dec 30, 2017

no, we should not. Please let's all work together on software instead of polluting it with so called "social issues" and politics of any kind. Please spare my area of work from left-wing ideas. Thanks.

@sgrif

This comment has been minimized.

Show comment
Hide comment
@sgrif

sgrif Dec 30, 2017

Member

@nsrsr Objecting to the term "insane" in a feature name in favor of another word is a completely reasonable request. It very much falls under "Using welcoming and inclusive language" which we've laid out in this project's values very clearly.

Attacking people for such ideas will not be tolerated. This is your only warning.

Member

sgrif commented Dec 30, 2017

@nsrsr Objecting to the term "insane" in a feature name in favor of another word is a completely reasonable request. It very much falls under "Using welcoming and inclusive language" which we've laid out in this project's values very clearly.

Attacking people for such ideas will not be tolerated. This is your only warning.

@diesel-rs diesel-rs deleted a comment from nsrsr Dec 31, 2017

@kochmaxence

This comment has been minimized.

Show comment
Hide comment
@kochmaxence

kochmaxence Jan 2, 2018

I'm closing this for several reasons:

  1. Bumping the allowed table size is a bad idea that originates from a bad idea, which will cause more drama in the long term: issues with "I need to support 150 columns". Large tables are a bad design choice.
  2. The increased compile time is subject to drama in issues.
  3. The drama around how to name things is driving me insane (pun intended).
  4. I opened this in Oct. and kept it open until now because I still don't feel like it's the correct approach.

-> 5. If anyone needs to arbitrarily increase the maximum size, it's better to let them fork & modify to their needs. Maybe we should just document it somewhere, I might just open a new PR with some documentation and snippets I used to auto-generate it.

That said, I feel like @nsrsr had a point (in his deleted comment) about the issues we bring into our open-source community from society. But it's not the place to discuss this.

kochmaxence commented Jan 2, 2018

I'm closing this for several reasons:

  1. Bumping the allowed table size is a bad idea that originates from a bad idea, which will cause more drama in the long term: issues with "I need to support 150 columns". Large tables are a bad design choice.
  2. The increased compile time is subject to drama in issues.
  3. The drama around how to name things is driving me insane (pun intended).
  4. I opened this in Oct. and kept it open until now because I still don't feel like it's the correct approach.

-> 5. If anyone needs to arbitrarily increase the maximum size, it's better to let them fork & modify to their needs. Maybe we should just document it somewhere, I might just open a new PR with some documentation and snippets I used to auto-generate it.

That said, I feel like @nsrsr had a point (in his deleted comment) about the issues we bring into our open-source community from society. But it's not the place to discuss this.

@kochmaxence kochmaxence closed this Jan 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment