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

mysqldump failing in tests #640

Closed
Tracked by #770
benjaminjb opened this issue Apr 14, 2021 · 5 comments · Fixed by #776
Closed
Tracked by #770

mysqldump failing in tests #640

benjaminjb opened this issue Apr 14, 2021 · 5 comments · Fixed by #776
Assignees
Labels
bug Something isn't working s: fixed was fixed or solution offered
Milestone

Comments

@benjaminjb
Copy link

benjaminjb commented Apr 14, 2021

Description

When submitting a PR, I noticed the mysql tests were failing remotely when no tests were failing locally. The error:

mysqldump: Couldn't execute 'SELECT COLUMN_NAME,                       JSON_EXTRACT(HISTOGRAM, '$."number-of-buckets-specified"')                FROM information_schema.COLUMN_STATISTICS                WHERE SCHEMA_NAME = 'pop_test' AND TABLE_NAME = 'addresses';': Unknown table 'COLUMN_STATISTICS' in information_schema (1109)

(cite: https://github.com/gobuffalo/pop/pull/639/checks?check_run_id=2326259652)

This seemed to be an error from a different version of mysqldump -- the Github action workflows specify mysql:5.7, but when I threw a print statement in to see what version of mysqldump we had, I got mysqldump Ver 8.0.23-0ubuntu0.20.04.1 for Linux on x86_64 ((Ubuntu)).
(cite: https://github.com/gobuffalo/pop/pull/639/checks?check_run_id=2326648018)

(When run locally/in an Ubuntu VM, the mysqldump version was different but always reference the 5.7 distribution.)

As this error seemed to be related to a newer version of mysqldump, I added the --column-statistics=0 option to the mysqldump commands, which solved the symptom.

Steps to Reproduce the Problem

  1. Remove --columnn-statistics=0 hack from this PR 3887167#diff-c866e330877c43db022d8a4d9ad8e554dab7562977944ec79cd792582b0d9543R160-R164
  2. Submit PR
  3. Check failed mysql test logs

Expected Behavior

Tests should pass with specified version of mysqldump

Actual Behavior

Tests should not fail

Info

Pop (through Buffalo): github.com/gobuffalo/pop/[email protected]
Go: go1.16.2 darwin/amd64
MacOS: 10.14.6 (18G6032)

@bryanjhv
Copy link

Having almost the same problem here, caused by the --column-statistics=0 addition.

It happens that MariaDB's mysqldump doesn't know about that option, so it fails saying:

mysqldump: unknown variable 'column-statistics=0'

Using:

  • mysqldump Ver 10.19 Distrib 10.5.10-MariaDB, for Linux (x86_64)
  • Buffalo Pop v5.3.4

@elboletaire
Copy link

elboletaire commented Oct 27, 2021

Same issue than @bryanjhv here. Is there any workaround? 🤔 Installing the official mysql client is not an option for me, as I'm under arch linux and, tbh, I do not want to build it from source (neither from AUR). (ok nevermind, I ended up just downloading an already built binary from the official mysql website).

Using:

  • mysqldump Ver 10.19 Distrib 10.6.4-MariaDB, for Linux (x86_64)
  • pop/v5 v5.3.4
  • buffalo v0.17.3

Edit: Anyway I'm thinking why is mysqldump a requirement? IMHO (and from my experience using other migration tools and frameworks) the sql calls should be made from the pop library itself, removing the binary dependency, because right now if you put this app on a container you'll need to install mysqldump in there to be able to run migrations 😕

@hxgdzyuyi
Copy link

Same issue than @bryanjhv here. Is there any workaround?

@sio4 sio4 added the s: triage Some tests need to be run to confirm the issue label Sep 20, 2022
@sio4 sio4 added this to the v6.0.7 milestone Sep 20, 2022
@sio4 sio4 added bug Something isn't working s: in progress Someone is working on this and removed s: triage Some tests need to be run to confirm the issue labels Sep 22, 2022
@sio4
Copy link
Member

sio4 commented Sep 22, 2022

Yeah, adding an option to the hard-coded command must be careful and need to check the oldest supported version. I rolled back the change in the code base and it will be released as v6.0.7 within a few weeks.

Meanwhile, one available workaround for the issue could be using a script. I didn't tested but roughly creating the following script as mysqldump and if you place it in the highest priority PATH, (for me ~/.bin) the it could be able to dump the schema without error.

#!/bin/bash

/usr/bin/mysqldump `echo $@ |sed 's/--column-statistics=0//'`

Didn't test, just imagined, so please test it before using it.

Edit: Anyway I'm thinking why is mysqldump a requirement? IMHO (and from my experience using other migration tools and frameworks) the sql calls should be made from the pop library itself, removing the binary dependency, because right now if you put this app on a container you'll need to install mysqldump in there to be able to run migrations confused

Hi @elboletaire, which SQL can be used to dump schema? Could you please guide me if you know how? or which tools are works like that? It could be better if there is a way, but I couldn't find the way yet.

@sio4 sio4 self-assigned this Sep 22, 2022
@sio4
Copy link
Member

sio4 commented Sep 22, 2022

One more point: when it runs in a production, dumping schema could be optional. Will check that in the next cycle.

@sio4 sio4 closed this as completed in #776 Sep 24, 2022
@sio4 sio4 added s: fixed was fixed or solution offered and removed s: in progress Someone is working on this labels Sep 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working s: fixed was fixed or solution offered
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants