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.

Sign up for GitHub

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

Jump to bottom

Implement CASE WHEN format #101

Merged
merged 3 commits into from
May 15, 2024
Merged

Implement CASE WHEN format #101

merged 3 commits into from
May 15, 2024

Conversation

mvorisek
Copy link
Contributor

@mvorisek mvorisek commented May 11, 2024 •
edited
Loading

closes #36

Supporting CASE WHEN SQL is important to format queries like https://github.com/atk4/data/blob/a9c6063420/tests/Persistence/Sql/QueryTest.php#L867 to be readable.

Please merge into 1.3.x as we need PHP 7.4 support.

@mvorisek mvorisek marked this pull request as draft May 11, 2024 17:52
@mvorisek mvorisek marked this pull request as ready for review May 11, 2024 18:13
@derrabus derrabus changed the base branch from 1.3.x to 1.4.x May 13, 2024 08:50
@derrabus
Copy link
Member

Please merge into 1.3.x as we need PHP 7.4 support.

We don't maintain 1.3 anymore. Please target 1.4.x.

derrabus
derrabus reviewed May 13, 2024
View reviewed changes
tests/format.txt Outdated
END
ELSE
40
END;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add tests with…

  • … a FROM clause?
  • … a column without a CASE statement after the one with the CASE statement?
  • … a column alias with AS for one of the columns built with a CASE statement?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

* … a `FROM` clause?

do you mean select case ... end from ...?

* … a column without a `CASE` statement after the one with the `CASE` statement?

what exactly do you mean?

* … a column alias with `AS` for one of the columns built with a `CASE` statement?

do you mean select case ... end AS xxx?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@derrabus I will do it, but let me please know what you want.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You understood me correctly already. 🙂

… a column without a CASE statement after the one with the CASE statement?

something like this:

SELECT some_column, CASE … END, some_other_column FROM some_table;

Copy link
Contributor Author

@mvorisek mvorisek May 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done and retargeted

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a rebase as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rebased

@mvorisek mvorisek force-pushed the case_when_format branch 2 times, most recently from 728aae1 to 43c8828 Compare May 13, 2024 10:23
@derrabus derrabus added the enhancement New feature or request label May 13, 2024
@mvorisek mvorisek force-pushed the case_when_format branch 3 times, most recently from 3f3bdc9 to 8e102fa Compare May 13, 2024 17:29
@mvorisek mvorisek force-pushed the case_when_format branch 2 times, most recently from 6e642b2 to ae2bf3e Compare May 13, 2024 22:27
greg0ire
greg0ire previously approved these changes May 14, 2024
View reviewed changes
Copy link
Member

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be squash-merged

@greg0ire
Copy link
Member

🤔 why are you targeting 1.4.x? This is not a bugfix, is it?

@mvorisek
Copy link
Contributor Author

There is no 1.5.x, if you will create it, feel free to retarget :)

@greg0ire greg0ire changed the base branch from 1.4.x to 1.5.x May 14, 2024 11:53
@greg0ire greg0ire dismissed their stale review May 14, 2024 11:53

The base branch was changed.

@derrabus derrabus modified the milestone: 1.5.0 May 14, 2024
@greg0ire greg0ire merged commit 2dbae95 into doctrine:1.5.x May 15, 2024
10 checks passed
@greg0ire
Copy link
Member

Thanks @mvorisek !

@mvorisek mvorisek deleted the case_when_format branch May 15, 2024 06:50
@derrabus derrabus changed the title Impl. CASE WHEN format Implement CASE WHEN format May 15, 2024
This was referenced May 18, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derrabus derrabus derrabus left review comments

@SenseException SenseException SenseException approved these changes

@greg0ire greg0ire greg0ire approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
1.5.0
Development

Successfully merging this pull request may close these issues.

Linebreak CASE… WHEN
4 participants
@mvorisek @derrabus @greg0ire @SenseException