Skip to content

Support cache when creating a sequence #11261

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

Open
Tracked by #22953
bruceliu2008 opened this issue Mar 14, 2018 · 14 comments
Open
Tracked by #22953

Support cache when creating a sequence #11261

bruceliu2008 opened this issue Mar 14, 2018 · 14 comments

Comments

@bruceliu2008
Copy link

Hi,
May I know are there any plan for ef core to support cache size for sequence, something like this:
modelBuilder.HasSequence("OrderNumbers", schema: "shared")
.StartsAt(1000)
.IncrementsBy(5)
.CacheSize(10);

Thanks,
Bruce'

@ajcvickers ajcvickers added this to the Backlog milestone Mar 14, 2018
@ajcvickers ajcvickers added type-enhancement help wanted This issue involves technologies where we are not experts. Expert help would be appreciated. labels Mar 14, 2018
@ilmax
Copy link
Contributor

ilmax commented Apr 2, 2018

If nobody is working on this, I would like to give it a try

@divega
Copy link
Contributor

divega commented Apr 3, 2018

Sounds good @ilmax. Just keep in mind that at this stage we may choose not to take the PR in 2.1.

@ilmax
Copy link
Contributor

ilmax commented Apr 3, 2018

Ok, no problem! I'm working on it now

@ilmax
Copy link
Contributor

ilmax commented Apr 3, 2018

I made some progress on this, some unit tests are still missing but the implementation should mostly be done. I have a some questions though and would like to get some feedback because adding cache size support ended up in a breaking change, I've also left a couple of TODOs around.
What's the best way to proceed with this? Should I open a WIP PR or wait till 2.1 is out of the door?

@smitpatel
Copy link
Contributor

@ilmax - Feel free to open PR. We can always merge it after 2.1 or merge into feature branch if 2.1 is not fully done.

@koenmetsu
Copy link

@ilmax if things are working out, could you try fitting in "NO CACHE" as well?

@divega divega added good first issue This issue should be relatively straightforward to fix. help wanted This issue involves technologies where we are not experts. Expert help would be appreciated. and removed help wanted This issue involves technologies where we are not experts. Expert help would be appreciated. good first issue This issue should be relatively straightforward to fix. labels May 31, 2019
@bricelam bricelam added the good first issue This issue should be relatively straightforward to fix. label May 31, 2019
@ajcvickers ajcvickers removed the help wanted This issue involves technologies where we are not experts. Expert help would be appreciated. label Aug 5, 2019
@Compufreak345
Copy link

@ajcvickers As you removed the help wanted tag - can you tell something about the current state of this issue? Thanks!

@ajcvickers
Copy link
Contributor

@Compufreak345 There were some inconsistencies in how help wanted was being used. This issue is marked as good first issue, indicating the implementation should not be too difficult for a community member to attempt.

Beyond that, this issue is in the Backlog milestone, which means we still plan to implement it in some future release, but not for the next release.

@bikbov
Copy link
Contributor

bikbov commented Oct 14, 2022

@ajcvickers @smitpatel Hi. I'd be happy to implement this feature.

sequence documentation for for sql server https://learn.microsoft.com/en-us/sql/t-sql/statements/create-sequence-transact-sql?view=sql-server-ver16

CREATE SEQUENCE [schema_name . ] sequence_name
[ AS [ built_in_integer_type | user-defined_integer_type ] ]
[ START WITH ]
[ INCREMENT BY ]
[ { MINVALUE [ ] } | { NO MINVALUE } ]
[ { MAXVALUE [ ] } | { NO MAXVALUE } ]
[ CYCLE | { NO CYCLE } ]
[ { CACHE [ ] } | { NO CACHE } ]
[ ; ]

Defaults to CACHE.
If the cache option is enabled without specifying a cache size, the Database Engine will select a size.

Cache statement will be added when translation to sql in the end.
CacheSize(long? cacheSize = null) - signature

There are two variants

  1. cache enabled, but cache size is not specifying
    modelBuilder.HasSequence("TSequence", schema: "shared")
    .CacheSize();
    will be translated to "CREATE SEQUENCE TSequence CACHE"

  2. cache enabled, cache size is specifying
    modelBuilder.HasSequence("TSequence", schema: "shared")
    .CacheSize(5);
    will be translated to "CREATE SEQUENCE TSequence CACHE 5"
    cacheSize = 1 corresponds to NO CACHE

@bikbov
Copy link
Contributor

bikbov commented Oct 17, 2022

Perhaps another function name would be appropriate

@roji
Copy link
Member

roji commented Oct 17, 2022

cacheSize = 1 corresponds to NO CACHE

In SQL Server, it's possible to use both CACHE 1 and NO CACHE - I think we should allow the user to express both via the Fluent API (even if we're not aware of a functional difference).

Also, our Fluent API methods generally start with a verb (e.g. UseThis, HasThat). So I think we should have:

.UseCache(5) // CACHE 5
.UseCache() // CACHE (default)
.UseNoCache() // NO CACHE

Finally, note that SQL Server, PostgreSQL and Oracle support this option, so we should add support in Relational (and not just in the SQL Server provider).

@bikbov
Copy link
Contributor

bikbov commented Oct 18, 2022

Great! Thanks a lot for feedback

@ajcvickers ajcvickers added customer-reported and removed good first issue This issue should be relatively straightforward to fix. labels Jan 27, 2024
@ajcvickers ajcvickers modified the milestones: Backlog, 9.0.0 Jan 27, 2024
@ajcvickers ajcvickers modified the milestones: 9.0.0, 9.0.0-preview1, 9.0.0-preview2 Jan 31, 2024
@ajcvickers ajcvickers modified the milestones: 9.0.0-preview2, Backlog Jul 17, 2024
@ajcvickers
Copy link
Contributor

Reverting this from 9. We will revisit in the future and agree on how it should be implemented.

@ajcvickers ajcvickers reopened this Jul 17, 2024
ajcvickers added a commit that referenced this issue Jul 17, 2024
ajcvickers added a commit that referenced this issue Jul 18, 2024
* Revert "impements #11261 sequence cache support"

This reverts commit 73904c0.

* Updating baselines.
@fsbflavio
Copy link

Is it possible to include an option for "ORDER" and "NOORDER"?. Oracle has this option too:

CREATE SEQUENCE [schema_name . ] sequence_name
[ AS [ built_in_integer_type | user-defined_integer_type ] ]
[ START WITH ]
[ INCREMENT BY ]
[ { MINVALUE [ ] } | { NO MINVALUE } ]
[ { MAXVALUE [ ] } | { NO MAXVALUE } ]
[ORDER | { NOORDER }]
[ CYCLE | { NO CYCLE } ]
[ { CACHE [ ] } | { NO CACHE } ]
[ ; ]

https://docs.oracle.com/en/database/oracle/oracle-database/19/sqlrf/CREATE-SEQUENCE.html

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

Successfully merging a pull request may close this issue.