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

Sphinx Connections #221

Closed
martingallagher opened this issue Mar 22, 2014 · 12 comments
Closed

Sphinx Connections #221

martingallagher opened this issue Mar 22, 2014 · 12 comments
Labels
Milestone

Comments

@martingallagher
Copy link

I'm investigating connecting to Sphinx with this package, instead of relying on the butchered SphinxQL package (https://github.com/Mutatio/sphinxql) which is no longer maintained.

I believe the issue could be MySQL version string handling. When calling Ping() I found it hangs in buffer.go in fill() at "n, err := b.rd.Read(b.buf[b.length:])" (line 55).

Doing a simple println(string(b.buf[b.length:])) returns "5.6.0" and some trailing bytes. This value coincides with the MySQL version string emitted by the Sphinx search daemon: mysql_version_string = 5.6.0

@julienschmidt
Copy link
Member

The code of the sphinxql package is heavily outdated and contains a lot of bugs which are fixed in the mysql package.

I think @arnehormann and I are both not active Sphinx users and have very little experience with it. We would like to provide a working package someday, but currently we don't have the time to maintain it.

We plan to move the wire protocol stuff of the mysql package into a subpackage, which then could be reused by other packages like the sphinxql package. This hopefully reduces the effort of maintaining the package. But I think we need people for that, who actively use it.

Regarding the buffer fill problem: This sounds a lot like the server is sending a wrong (too high) packet length value in the packet header (packets.go:38). The client then waits forever for the remaining data (packets.go:57).

The mysql package currently avoids handling the version string, these bytes are skipped by searching for the index of the next string null termination 0x00: (packets.go:149).

@martingallagher
Copy link
Author

Thanks for the quick response. The hints sound promising; hopefully over the next few days I can look into this a bit deeper. Sadly it sounds like non-standard behaviour on Sphinx's part.

@julienschmidt
Copy link
Member

This sounds like a bug, I haven't heard of non-standard behavior of Sphinx yet

@Terry-Mao
Copy link

sphinx do not support MySQL Prepare , the database/sql auto use prepare...

@julienschmidt
Copy link
Member

Yes, this is why there is an (unmaintained) SphinxQL package. But you should be able to use the MySQL driver when you manually build query strings without parameters. The driver then uses the text protocol, which is supported by Sphinx.

@xaprb
Copy link

xaprb commented Mar 23, 2014

Is it worth mentioning Sphinx and MemSQL as examples of a special case in
the go-database-sql tutorial? The special case I have in mind is a product
that can be used with database/sql but doesn't support prepared statements,
and how to avoid creating prepared statements.

@julienschmidt
Copy link
Member

For now, yes. How? Just don't use query parameters (use fmt.Sprintf etc. to build a single query string, but beware of SQL injections!).

Eventually we will revive the SphinxQL package or add a parameter to the mysql package to enforce the usage of the text protocol.

@julienschmidt
Copy link
Member

It might be helpful if this driver had some exported helper functions to escape strings

@martingallagher
Copy link
Author

I'm not convinced this is a PS issue, this is more the initial handshake on db.Ping(). On my patched version (https://github.com/Mutatio/sphinxql) of the Sphinx package there was also a erroneous column count on SELECT queries. The applied patch is incredibly naive and just passes the column count as-is without validation. It's worth noting that the Sphinx package correctly emits a error when attempting PS to the affect "Sphinx doesn't support prepared statements".

@martingallagher
Copy link
Author

There are further developments here: http://sphinxsearch.com/forum/view.html?id=11339

The proposed tweaks allow me to connect to Sphinx-trunk; it appears Sphinx might need some tweaks.

@julienschmidt
Copy link
Member

To sum up, 2 problems:

  • Sphinx does not support select @@max_allowed_packet correctly
  • Sphinx appends an extra 0x00 byte to the column_count LengthEncodedInteger

@julienschmidt
Copy link
Member

Fixed in the newest Sphinx version! https://plus.google.com/113485961989931426612/posts/ZugAFqQiLK4

Should we add Sphinx 2.2.3+ to the clients list in the requirements section?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants