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

3.5.6: Archive workflow sql syntax error with mariadb #13202

Closed
4 tasks done
spaced opened this issue Jun 17, 2024 · 3 comments · Fixed by #13203
Closed
4 tasks done

3.5.6: Archive workflow sql syntax error with mariadb #13202

spaced opened this issue Jun 17, 2024 · 3 comments · Fixed by #13203
Labels
area/server area/workflow-archive P2 Important. All bugs with >=3 thumbs up that aren’t P0 or P1, plus: Any other bugs deemed important solution/suggested A solution to the bug has been suggested. Someone needs to implement it. type/bug type/regression Regression from previous behavior (a specific type of bug)

Comments

@spaced
Copy link
Contributor

spaced commented Jun 17, 2024

Pre-requisites

  • I have double-checked my configuration
  • I have tested with the :latest image tag (i.e. quay.io/argoproj/workflow-controller:latest) and can confirm the issue still exists on :latest. If not, I have explained why, in detail, in my description below.
  • I have searched existing issues and could not find a match for this bug
  • I'd like to contribute the fix myself (see contributing guide)

What happened/what did you expect to happen?

since v3.5.6 using mariadb can not list workflows because of 200f4d1e5 / #12912

exception

{"error":"rpc error: code = Internal desc = Error 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '\u003e\u003e'$.metadata.labels', '{}') as labels,coalesce(workflow-\u003e\u003e'$.metadata.annota...' at line 5","grpc.code":"Internal","grpc.method":"ListWorkflows","grpc.service":"workflow.WorkflowService","grpc.start_time":"2024-06-17T14:57:19Z","grpc.time_ms":6.307,"level":"error","msg":"finished unary call with code Internal","span.kind":"server","system":"grpc","time":"2024-06-17T14:57:19.890Z"}
{"duration":6836621,"level":"info","method":"GET","msg":"","path":"/api/v1/workflows","size":292,"status":500,"time":"2024-06-17T14:57:19.890Z"}

because json extract with -> is not supported by mariadb (see also https://jira.mariadb.org/browse/MDEV-13594)
we should use JSON_EXTRACT(field,path)

Version

v3.5.6

Paste a small workflow that reproduces the issue. We must be able to run the workflow; don't enter a workflows that uses private images.

nope

Logs from the workflow controller

kubectl logs -n argo deploy/workflow-controller | grep ${workflow}

Logs from in your workflow's wait container

kubectl logs -n argo -c wait -l workflows.argoproj.io/workflow=${workflow},workflow.argoproj.io/phase!=Succeeded
@agilgur5 agilgur5 added the type/regression Regression from previous behavior (a specific type of bug) label Jun 17, 2024
@agilgur5 agilgur5 added this to the v3.5.x patches milestone Jun 17, 2024
@agilgur5 agilgur5 changed the title Archive workflow sql sytax error with mariadb 3.5.6: Archive workflow sql syntax error with mariadb Jun 17, 2024
@agilgur5
Copy link
Contributor

agilgur5 commented Jun 17, 2024

Thanks for filing an issue and giving instructions on how to resolve it!

This also got mentioned twice on Slack earlier today and last week.

cc @jiachengxu it appears we might need to add a case for MariaDB since this specific syntax is different compared to MySQL

@agilgur5 agilgur5 added solution/suggested A solution to the bug has been suggested. Someone needs to implement it. P2 Important. All bugs with >=3 thumbs up that aren’t P0 or P1, plus: Any other bugs deemed important labels Jun 17, 2024
@jiachengxu
Copy link
Member

cc @jiachengxu it appears we might need to add a case for MariaDB since this specific syntax is different compared to MySQL

@agilgur5 Thanks for tagging me. I just want to make sure that I understand the issue, argo-workflows only supports MySQL and Postgres, right? and this is more like a feature request?

@agilgur5
Copy link
Contributor

agilgur5 commented Jun 17, 2024

Yes. Well, kind of. MariaDB's fork of MySQL is supposed to be compatible, but I guess this is one of the few cases where it isn't(?) (I'm not sure if the JSON extraction is a newer syntax? Also we may need to update the workflow archive docs with the min version of MySQL? It's been quite some time since I've used either. EDIT: Looks like the >=5.7.8 that we already require is where the JSON data type and functions were added, so we're good on that front).

It wasn't officially supported before, correct.
But apparently it was working before and had a decent number of users, so if we can make it work again simply, that'd be great (especially since it broke within a patch version vs a minor). I think this is the only case where the syntax would be different.

EDIT: OP wrote up a PR with a compatible syntax that works on both: #13203

agilgur5 pushed a commit that referenced this issue Jul 6, 2024
…riaDB. Fixes #13202 (#13203)

Signed-off-by: spaced <[email protected]>
(cherry picked from commit ac9cb3a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/server area/workflow-archive P2 Important. All bugs with >=3 thumbs up that aren’t P0 or P1, plus: Any other bugs deemed important solution/suggested A solution to the bug has been suggested. Someone needs to implement it. type/bug type/regression Regression from previous behavior (a specific type of bug)
Projects
None yet
3 participants