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

Add description for quickly running/testing custom SQL statements #244

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

klauck
Copy link
Contributor

@klauck klauck commented Jun 21, 2024

The possibility for quickly running/testing SQL statements with the parser is not documented in the README.

Some issues, such as #243, could, for example, described and tested with the command line executable ./example/example.

This pull request documents this feature.

Add a description for quickly running/testing custom SQL statements
@klauck
Copy link
Contributor Author

klauck commented Jun 21, 2024

I am not sure where to place this documentation:

  1. Under the example project (as is)
  2. As a separate feature

1 makes sense, because compilation is also required and the code is referenced
2 makes sense, because the executable can be used to test statements without having an own project

@klauck klauck requested review from Bouncner and dey4ss June 21, 2024 11:17
@Bouncner
Copy link
Contributor

I was not even aware of the example binary. Can we find a better name? Repl? Console?

I am not sure where to place this documentation:

What documentation do you mean exactly?

@dey4ss
Copy link
Member

dey4ss commented Jun 21, 2024

Can we find a better name? Repl? Console?

It's not interactive, you just provide the query as CLI argument and it prints it. I guess the name comes from the fact that it's basically a code snippet that shows how to use the parser in an application – with the side effect of printing the statement. But I don't have a better idea for a name

@klauck
Copy link
Contributor Author

klauck commented Jun 21, 2024

I was not even aware of the example binary

Yeah, and it is a great possibility to try out the parser. This is why I would like to add a documentation of this feature in the README.

What documentation do you mean exactly?

The proposed change of the README. My question is how to integrate the change in the existing README.
The current proposal is to put it as point 7. under "To use the SQL parser in your own projects you simply have to follow these few steps." I think this place is not a perfect fit.

Maybe, It is better to separate the build and test process 1.-4. and then have the option to

  1. Use it as a library

or 2. try it out running ./example/example

Separate build and usage documentation
@klauck
Copy link
Contributor Author

klauck commented Jun 26, 2024

I separated the build and usage documentation. Please let me if you have further recommendations.

Copy link
Contributor

@Bouncner Bouncner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few questions and suggestions. I think the proposed structure makes sense and is much better to read.

@@ -13,37 +13,58 @@ In March 2015 we've also written a short paper outlining discussing some develop

**Note:** You can also find a detailed usage description [here](docs/basic-usage.md).

To use the SQL parser in your own projects you simply have to follow these few steps.

### Build and Test

1. Download the [latest release here](https://github.com/hyrise/sql-parser/releases)
2. Compile the library `make` to create `libsqlparser.so`
3. *(Optional, Recommended)* Run `make install` to copy the library to `/usr/local/lib/`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not yours, but I wouldn't consider it "recommended" to install the library. I would be rather surprised if somebody installs it system-wide.

@@ -13,37 +13,58 @@ In March 2015 we've also written a short paper outlining discussing some develop

**Note:** You can also find a detailed usage description [here](docs/basic-usage.md).

To use the SQL parser in your own projects you simply have to follow these few steps.

### Build and Test

1. Download the [latest release here](https://github.com/hyrise/sql-parser/releases)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, well, well ...

image

@@ -13,37 +13,58 @@ In March 2015 we've also written a short paper outlining discussing some develop

**Note:** You can also find a detailed usage description [here](docs/basic-usage.md).

To use the SQL parser in your own projects you simply have to follow these few steps.

### Build and Test

1. Download the [latest release here](https://github.com/hyrise/sql-parser/releases)
2. Compile the library `make` to create `libsqlparser.so`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the library USING make ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.so is linux-only. Just say "to create the library?

{
// Basic Usage Example

const std::string query = "...";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To annoy people, I would use auto-to-stick here. 🫣


5. The build `example` executable can be used to test/parse specific custom SQL statements:

Running, for example, `./example/example "SELECT * FROM test;"` produces:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parse_simple_statement as the binary name? Any ideas?

}
}
```

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last line in README: HYRISE > Hyrise

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