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

added fix for non utf8 query event #146

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mastak
Copy link
Contributor

@mastak mastak commented Mar 11, 2016

I faced with this #132 problem.

Does this solution have a chance to live? (=

@julien-duponchelle
Copy link
Owner

What is the schema of the column creating the issue?

@bjoernhaeuser
Copy link
Collaborator

@mastak can you add a test which show cases the error (and the fix)?

@mastak
Copy link
Contributor Author

mastak commented Jul 8, 2016

Sorry, guys, that I'm so slowly.
The column is simple varchar, the table has latin-1 charset.
I wrote some test, but it is work only with STATEMENT based replication, and I don't know how it can be enabled. Can you help with it?

    def test_encoding_latin1_statement(self):
        db = copy.copy(self.database)
        db["charset"] = "latin1"
        self.connect_conn_control(db)

        create_query = "CREATE TABLE test (test CHAR(12)) CHARACTER SET latin1 COLLATE latin1_bin;"
        insert_query = b"INSERT INTO test VALUES('\x96')"

        self.execute(create_query)
        self.execute(insert_query)
        self.execute("COMMIT")

        self.assertIsInstance(self.stream.fetchone(), RotateEvent)
        self.assertIsInstance(self.stream.fetchone(), FormatDescriptionEvent)
        # QueryEvent for the Create Table
        self.assertIsInstance(self.stream.fetchone(), QueryEvent)

        # QueryEvent for the BEGIN
        self.assertIsInstance(self.stream.fetchone(), QueryEvent)

        event = self.stream.fetchone()
        self.assertIsInstance(event, QueryEvent)

        self.assertEqual(event.query, insert_query.decode("latin-1"))

@mastak
Copy link
Contributor Author

mastak commented Jul 11, 2016

Could we do something like:
at the begin of the test SET GLOBAL binlog_format = 'STATEMENT';
at the end of the test SET GLOBAL binlog_format = 'ROW';
?

@mastak mastak force-pushed the dev-non-utf8 branch 4 times, most recently from 854851e to 9ba26bc Compare July 13, 2016 07:40
@julien-duponchelle
Copy link
Owner

Clearly the fix work but will break it's non utf-8 and non latin1.

But what do you think about @baloo solution in #159 to just return a byte object so you can handle it in your code?

@mastak
Copy link
Contributor Author

mastak commented Jul 13, 2016

Sure, it would be good.

@mastak
Copy link
Contributor Author

mastak commented Jul 13, 2016

We can't return bytes (even only in case with error) because it will break existing systems.

But what if add some method like:

def _decode_query(self, query)
    return query.decode("utf-8")

and then everyone can overwrite it. What do you think?

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.

None yet

3 participants