Remove Postgrex in favor of epgsql about moebius HOT 13 CLOSED

Postgrex has a feature called type extensions which is exactly for cases like this. You can configure how encode and decoding works in any way you prefer. Postgrex is restrictive because Postgres requires explicitly casting but you can change that. That's what Ecto uses for example to treat Postgrex dates internally as tuples and bypass the dates.

from moebius.

Many of those issues could be solved just by communicating and asking. Ask the Postgrex team, it doesn't hurt. Maybe it won't be enough by the end of the day, but at least you won't go down reinventing the wheel unless you are certain you need to.

from moebius.

I did. And I'm not reinventing anything, using something that's been around for years. More later on a train atm.

On Jan 11, 2016, at 18:09, José Valim [email protected] wrote:

Many of those issues could be solved just by communicating and asking. Ask the Postgrex team, it doesn't hurt. Maybe it won't be enough by the end of the day, but at least you won't go down reinventing the wheel unless you are certain you need to.

—
Reply to this email directly or view it on GitHub.

from moebius.

I know you are planning to use epgsql, that's fine. My comment was related
to your mention of forking and writing your own, which is insane
considering only the type stuff is the one causing issues (at least
according to this issue).

Please point me to the discussion related to Postgrex and moebius. I would
love to know more details on why the extension system was not enough.

José Valimwww.plataformatec.com.br
http://www.plataformatec.com.br/Founder and Director of R&D

from moebius.

Experimenting and not doing what I'm told are kind of my calling card :). Seriously though- when I get to better wifi I'll send more detail. I'm attracted to long running solutions.

On Jan 11, 2016, at 18:22, José Valim [email protected] wrote:

I know you are planning to use epgsql, that's fine. My comment was related
to your mention of forking and writing your own, which is insane
considering only the type stuff is the one causing issues (at least
according to this issue).

Please point me to the discussion related to Postgrex and moebius. I would
love to know more details on why the extension system was not enough.

José Valimwww.plataformatec.com.br
http://www.plataformatec.com.br/Founder and Director of R&D
—
Reply to this email directly or view it on GitHub.

from moebius.

Extending a driver to help it figure out dates and types is basically building the driver itself. Throwing because an atom comes your way is a tad confounding, and not allowing strings for date types is a true head scratcher. Postgres string translation and inference is so robust, and Postgrex stands squarely in the way of that. I can't right now ask for a date "< now() + 2 days", for instance because it's not a Postgrex time stamp.

When you see enough of these decisions... Yeah you get a little mental.

On Jan 11, 2016, at 18:01, José Valim [email protected] wrote:

Postgrex has a feature called extensions which is exactly for cases like this. You can configure how encode and decoding works in any way you prefer. Postgrex is restrictive by default but you can change that. That's what Ecto uses for example to treat Postgrex dates internally as tuples and bypass the dates.

—
Reply to this email directly or view it on GitHub.

from moebius.

Extending a driver to help it figure out dates and types is basically
building the driver itself.

That's just inaccurate. Of all concerns in the driver, implementing the
over the wire protocol, managing connections, authentication, etc. types is
just a small part. So no, figuring out types is not building the driver.

In fact, Postgrex type stuff is in a single file. You can copy it to your
project, change it to encode however you want and tell Postgrex to use it.

Postgres string translation and inference is so robust, and Postgrex stands
squarely in the way of that. I can't right now ask for a date "< now() + 2
days", for instance because it's not a Postgrex time stamp.

Afaik, once you talk to postgresql over the binary protocol, it becomes more
restrict. That's why you can't do things Iike: "SELECT $1 + $2". So in your
example above, postgresql itself would expect a date, exclusively. I don't
think you would be able to send a string in your example if it is
parameterized. I'd need to confirm this.

Also, atoms are not strings in Elixir. We are very explicit about it.
Treating them or converting them behind the scenes would just be unnatural.

from moebius.

Afaik, once you talk to postgresql over the binary protocol, it becomes more
restrict. That's why you can't do things Iike: "SELECT $1 + $2". So in your
example above, postgresql itself would expect a date, exclusively.

I have just confirmed this is exactly how postgresql works when you are talking
to it over the tcp binary protocol. So the decisions that are making you little mental
are coming directly from postgres. In this case, you can't send a string where
a date is expected. You would first need to parse and convert it explicitly to a type,
which is exactly what postgrex allows you to do with the extensions mechanism.

from moebius.

I appreciate that you want me to continue using Postgrex. I'd be happy to fork it and send PRs but I a) don't want to support a driver and b) don't have time to debate things that seem completely obvious to me.

Yeah I know an atom isn't a string. Converting things "behind the scenes" is precisely the job of a database driver (among others) and I'm wondering WTF else an atom could be when it comes to a database? Do you really need to throw here? Is this really an I can't go on let's halt execution kind of thing? I really don't think so.

I really don't think I need to rewrite SQL because Postgrex doesn't know what to do with inet, tsvector, [any user defined type] when I run a select *. That's what happens now and it's very annoying.

In this case, you can't send a string where a date is expected

Yes, you can. In Node (and .NET and Ruby) I'm able to do this nicely:

var pg = require("pg");

pg.connect("postgres://rob@localhost/redfour", function(err,client, done){
    client.query("insert into solar_flares(classification, scale, date) values ('x', 10.0, $1);", ["12/12/2003"], function(err, res){
     console.log(err); //null
     console.log(res); //command result
    done();
  });
});

I can't do that with Postgrex because $1 is not a Postgrex.Timestamp.

I don't want to write extensions for something that, to me, is completely obvious.

from moebius.

I appreciate that you want me to continue using Postgrex.

I don't necessarily want you to continue using Postgrex. All I was saying is that your problem has a solution which does not require you to rewrite the driver or move anywhere. It was a suggestion.

don't have time to debate things that seem completely obvious to me.

:( It is a pity you don't want to debate what is obvious because that's a perfect situation for teaching: making what is obvious to you become obvious to others. Someone could even implement timex_postgres that would solve those issues for everyone using timex, by simply adding one dependency. But if everyone is too busy to explain what they consider obvious, then progress won't happen.

Yes, you can. In Node (and .NET and Ruby) I'm able to do this nicely:

I didn't say you cannot ever do it. All I said is that: Postgres requires explicit casting for binary protocols. The driver is mapping close to Postgres semantics. You can still do implicit casting in Postgrex by using extensions.

I am stepping out of this because I don't think I am being helpful. But this goes back to the point I made in another issue: the reason I was commenting here is because I have heard folks interested using moebius in production and the database tool is extremely important to get right otherwise you have memory leaks, race conditions that corrupt data and overly relaxed casting rules that may introduce security issues (I can think of at least 3 Rails security releases because of casting).

If moebius is still largely an experiment, I think it would be nice to let potential users know they are joining an experimental project. Heck, the proposed pool implementation had a race condition that locked the connection inside a transaction, leading all following SQL commands to a transaction that would never commit. This would last for days until the server was restarted, ultimately causing data loss. If someone is deploying a proof of concept with Elixir and runs into such issue, it is almost guaranteed they won't be using Elixir again.

from moebius.

the database tool is extremely important to get right otherwise you have memory leaks, race conditions that corrupt data and overly relaxed casting rules that may introduce security issues (I can think of at least 3 Rails security releases because of casting).

This isn't my first time building one of these, José.

If moebius is still largely an experiment, I think it would be nice to let potential users know they are joining an experimental project. Heck, the proposed pool implementation had a race condition that locked the connection inside a transaction, leading all following SQL commands to a transaction that would never commit. This would last for days until the server was restarted, ultimately causing data loss. If someone is deploying a proof of concept with Elixir and runs into such issue, it is almost guaranteed they won't be using Elixir again.

Johnny's stuff is very raw and, for the exact reason you mention I am still not a fan of connection pools. I know you think they're important, but I have no failure metrics to measure it with other than the ridiculous OTP experiments I've been doing to blow it up.

To be honest I concentrate much more on the API end of things. Right now, to work with dates, Postgrex is punching a hole right in our abstraction. I've forked the driver and am trying to resolve how to do it - and yes I do understand how Postgres requires parameter casts (I'm running face-first into it now) - but other drivers in other languages have somehow gotten this right.

A driver shouldn't throw because it can't resolve something. That's my biggest issue. Postgrex is throwing on a number of occasions (as outlined above) and it's driving me a little crazy because I can't get around it. I'd like to solve this problem and right now the best way I can see to do it is to move to a driver that's been around for a while.

Finally - I'm happy to talk about this more, but this is the 3rd time you've suggested (tangentially) that Moebius (basically) shouldn't be used because it's not "ready". I understand the connection thing - but as you'll notice it's not in master and neither is Poolboy. I haven't seen enough failures to mandate a change and I doubt I will.

Turning an atom into a string is not a "relaxed casting issue", neither is handing off a string as a date. Postgres' type inference is unbelievably good - and if the string cast doesn't work handle the error and give it back. I can't see how this could lead to security issues but sure, I'm all ears on that. As I mention, other drivers do this.

I know I can be frustrating; I do appreciate you dropping in :). Let's see if we can stay on target.

from moebius.

This is my last comment. I promise. But I will be mostly rehashing what I have already said.

Johnny's stuff is very raw and, for the exact reason you mention I am still not a fan of connection pools.

Well distributed systems are also very hard. But some of its problems have been elegantly solved on the platform you are using. We have good solutions around. Poolboy has been extensibly tested in production by databases like Riak. Folks have built property check models to validate its concurrency. I don't understand how "not trusting pools" becomes "writing your own pool". All of the attempts at solving this problem in Moebius were pools. The GenServer was a pool, the "broker" was a pool, etc.

I've forked the driver and am trying to resolve how to do it

Just to be absolutely clear: you don't have to fork the driver as you can use the extensions API.

A driver shouldn't throw because it can't resolve something

It has to throw for two reasons: PostgreSQL supports custom types and we use the binary protocol. For example, if someone adds a PostgreSQL extension, it may add its own types and the best thing to do is raise so you can act on it. If we just pass data through, you will get gibberish. For example, here is how a timestamp is received from the binary protocol:

decode(<<0, 1, 204, 31, 244, 192, 51, 192>>)
#=> {{2016, 1, 12}, {11, 16, 39, 0}}

I can't see any situation where allowing this data to reach the user would be a good thing. You have to tell the driver how to decode it. And on encoding, you need to know how to convert A to B because it expects a explicit binary format. Those could be customised per application or something that a library, like Moebius, would resolve for you.

Finally - I'm happy to talk about this more, but this is the 3rd time you've suggested (tangentially) that Moebius (basically) shouldn't be used because it's not "ready". I understand the connection thing - but as you'll notice it's not in master and neither is Poolboy

I believe Moebius should have a disclaimer. Folks are of course free to use it, that considered. And I believe Moebius' current master is also not ready because it still does not follow the basic OTP patterns which is to start connection processes as children of a supervisor. If you are not supervising, it is easier to leak connections, you can't easily trace or introspect them, etc. I have raised this back in #37. Those are my beliefs and suggestions, nobody is required to follow them.

Erlang and Elixir ships with a bunch of great tools to help introspect and understand your system and they are not available to Moebius. But as promised, I will retreat from the discussion because at the end of the day I am not adding anything that I have not already said.

from moebius.

I'll put a warning label on Moebius if you put one on Postgrex. Deal?

from moebius.