Skip to content

Conversation

@niconoe-
Copy link
Contributor

Fixes #306 as ":=" was not recognized as an operator just like "=" was.

@niconoe- niconoe- changed the base branch from master to 5.8.x June 21, 2023 15:33
@niconoe-
Copy link
Contributor Author

@williamdes This time, I started my branch from 5.8.x 😉

@niconoe-
Copy link
Contributor Author

@williamdes, about the 2 failing tests, I have the feeling that's not on my hands: could it be the 5.8.x branch already having the issue even without my fix?

Maybe another PR is required before this one (and then, I can rebase) to fix those unit tests?

Thanks.

@williamdes williamdes closed this Jun 22, 2023
@williamdes williamdes reopened this Jun 22, 2023
@williamdes williamdes changed the title Fix Syntax error in subquery when used in SET #306 Fix #306 - ":=" was not recognized as an operator just like "=" Jun 22, 2023
@codecov
Copy link

codecov bot commented Jun 22, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (88c8314) 97.02% compared to head (0076001) 97.02%.

Additional details and impacted files
@@            Coverage Diff            @@
##              5.8.x     #486   +/-   ##
=========================================
  Coverage     97.02%   97.02%           
  Complexity     2234     2234           
=========================================
  Files            69       69           
  Lines          5144     5144           
=========================================
  Hits           4991     4991           
  Misses          153      153           
Impacted Files Coverage Δ
src/Components/ArrayObj.php 100.00% <ø> (ø)
src/Components/IndexHint.php 100.00% <ø> (ø)
src/Components/SetOperation.php 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@williamdes
Copy link
Member

@williamdes, about the 2 failing tests, I have the feeling that's not on my hands: could it be the 5.8.x branch already having the issue even without my fix?

Maybe another PR is required before this one (and then, I can rebase) to fix those unit tests?

Thanks.

You need to get the target branch right the first time or the CI will be run with the branch of your PR merged into the target branch
That's what created this inconsistent output

Closing and re-opening clears up the CI 🎉

@williamdes williamdes added this to the 5.9.0 milestone Jun 22, 2023
@niconoe-
Copy link
Contributor Author

@williamdes, about the 2 failing tests, I have the feeling that's not on my hands: could it be the 5.8.x branch already having the issue even without my fix?
Maybe another PR is required before this one (and then, I can rebase) to fix those unit tests?
Thanks.

You need to get the target branch right the first time or the CI will be run with the branch of your PR merged into the target branch That's what created this inconsistent output

Closing and re-opening clears up the CI tada

I'm not sure I get it 😆 . I started from the up-to-date 5.8.x, and the target branch is the same, so I don't understand why there's such issue.

Even with the current failing test, is this OK for you? Or do you need me to fix something?

@williamdes
Copy link
Member

I'm not sure I get it . I started from the up-to-date 5.8.x, and the target branch is the same, so I don't understand why there's such issue.

You did open the PR with target to master, so GitHub did an automatic merge with master and ran the CI on the result.

master branch renamed properties to camelCase. That's how I understood what got wrong

Even with the current failing test, is this OK for you? Or do you need me to fix something?

I only see green now 👼🏻

@williamdes williamdes self-assigned this Jun 29, 2023
@williamdes williamdes merged commit 0780376 into phpmyadmin:5.8.x Jun 29, 2023
@niconoe- niconoe- deleted the fix-306 branch June 29, 2023 10:09
williamdes added a commit that referenced this pull request Jun 29, 2023
Pull-request: #486

Signed-off-by: William Desportes <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Syntax error in subquery when used in SET

2 participants