Skip to content
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

Simplify calls to date nested in strftime #25027

Closed
roji opened this issue Jun 3, 2021 · 5 comments · Fixed by #28928
Closed

Simplify calls to date nested in strftime #25027

roji opened this issue Jun 3, 2021 · 5 comments · Fixed by #28928
Assignees
Labels
area-query area-sqlite closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. good first issue This issue should be relatively straightforward to fix. type-enhancement
Milestone

Comments

@roji
Copy link
Member

roji commented Jun 3, 2021

For example, strftime('%Y', date($date, '2 years')) can be simplifed to strftime('%Y', $date, '2 years').

(originally #24989 (comment))

@AndriySvyryd AndriySvyryd added this to the Backlog milestone Jun 4, 2021
@bricelam bricelam added the good first issue This issue should be relatively straightforward to fix. label Jun 4, 2021
@bricelam
Copy link
Contributor

bricelam commented Jun 4, 2021

Note to implementor: This requires updating the logic inside SqliteExpression.Strftime to handle date in addition to strftime:

&& rtrimFunction2.Arguments[0] is SqlFunctionExpression strftimeFunction
&& strftimeFunction.Name == "strftime"
&& strftimeFunction.Arguments!.Count > 1)
{
// Use its timestring parameter directly in place of ours
timestring = strftimeFunction.Arguments[1];
// Prepend its modifier arguments (if any) to the current call
modifiers = strftimeFunction.Arguments.Skip(2).Concat(modifiers);
}
var finalArguments = new[] { sqlExpressionFactory.Constant(format), timestring }.Concat(modifiers);

We should also think about whether it's possible to get nested date functions.

@jhowlett-scottlogic
Copy link

Is anyone working on this?

@bricelam
Copy link
Contributor

@jhowlett-scottlogic Nope, feel free.

@jhowlett-scottlogic
Copy link

Sorry @bricelam but i have looked into this task more deeply and have found it to be above my experience level with efcore. Because of this i don't feel like i should take on this issue as i do not fully understand the repository and may not take everything required into account when designing a solution.

I hope you find someone to take on this task soon!

@bricelam
Copy link
Contributor

@jhowlett-scottlogic No worries. But also feel free to ask us questions; we're always eager to help contributors get started. We have a few docs like How to contribute and Getting and Building the Code, but I'm sure we could use more.

bricelam added a commit to bricelam/efcore that referenced this issue Aug 31, 2022
Introducing SqliteSqlExpressionFactory as discussed in PR dotnet#23193

Fixes dotnet#25027
@bricelam bricelam added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Aug 31, 2022
bricelam added a commit that referenced this issue Aug 31, 2022
Introducing SqliteSqlExpressionFactory as discussed in PR #23193

Fixes #25027
@bricelam bricelam modified the milestones: MQ, 7.0.0-rc2 Aug 31, 2022
@ajcvickers ajcvickers modified the milestones: 7.0.0-rc2, 7.0.0 Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-query area-sqlite closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. good first issue This issue should be relatively straightforward to fix. type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants