memsql / dbbench

Database Benchmark Tool
Apache License 2.0
156 stars 32 forks source link

Add url option #14

Closed fjammes closed 3 years ago

fjammes commented 3 years ago

Add url option, so that dbbench can be invoked this way:

dbbench --url mysql://user:pass@host:port dbbench.ini
fjammes commented 3 years ago

Hi @aevernon! would it be please possible to merge this PR?

aevernon commented 3 years ago

Hello, I don't have write access to the repo anymore, but @awreece can help you.

awreece commented 3 years ago

Overall, it looks fine, but --I have a bit more philosophical question here. Why did we go down the route of a URL vs a DSN (e.g. using https://pkg.go.dev/github.com/go-sql-driver/mysql#ParseDSN)?

I don't have as much context about this world as I used to: are they equivalent? Is there an RFC or something that explains the correspondence?

awreece commented 3 years ago

Another question: If a user specificies:

dbbench --url mysql://user:pass@host:port --port 1234 test.ini

What do they expect is going to happen?

I can see a number of options (e.g. last flag win, or most specific flag wins), but I suspect "URL always wins" is probably not what they want. Is there a way to make this more intuitive?

fjammes commented 3 years ago

Hi @awreece , being able to manager both DSN and connection string looks more flexible to me, as some user would prefer connection string over DSN. It might depend of what you have as input configuration parameters.

About you second question (dbbench --url mysql://user:pass@host:port --port 1234 test.ini), as said in inline documentation:

dbbench --help
...
-url string
        Connection url (mysql://user:pass@host:port?params), parameters provided here override those provided by other options

the port parameter value will be "1234" as no port parameter is provided inside the URL.

Does this work for you please?

fjammes commented 3 years ago

Hi @awreece, could you please merge the PR when possible so that I can use this new option?

antiguru commented 3 years ago

It seems this PR broke Postgres connections: #15