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 interface to return queries by oracles #648

Merged
merged 2 commits into from
Dec 17, 2022
Merged

Add interface to return queries by oracles #648

merged 2 commits into from
Dec 17, 2022

Conversation

bajinsheng
Copy link
Collaborator

For QPG, we need to obtain the queries that are returned from oracles to measure query plans.

@bajinsheng bajinsheng linked an issue Dec 16, 2022 that may be closed by this pull request
Copy link
Contributor

@mrigger mrigger left a comment

Choose a reason for hiding this comment

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

I think we should aim for a more general solution here. Kareem recently added a class that can be used to return reproducers from oracles:

boolean bugStillTriggers(G globalState);

We could add a method String[] getQueriesAsString() that returns the test oracle queries as strings (QPG would only use one of them). For example, the constructor could take an array containing the optimizedQueryString and unoptimizedQueryString:

reproducer = new SQLite3NoRECReproducer(optimizedQuery, unoptimizedQuery);

We could create the reducer every time check() is called, not only when a bug is encountered. I think this solution would be more elegant and general (especially considering that most methods currently return a "Not implemented!" string). What do you think about this?

@@ -52,7 +52,7 @@ public Reproducer<G> generateAndTestDatabase(G globalState) throws Exception {

} catch (AssertionError e) {
Reproducer<G> reproducer = oracle.getLastReproducer();
if (reproducer != null) {
if (reproducer.getReproducerState()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason why need this method? I think it would be better to check for null: either we have the reproducer, or it is null. Having a reproducer object that indicates that we don't have the actual state is a bit confusing, I think.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

reproducer is returned by each call of check, instead of bug only, so we need to distinguish the returned reproduer denotes a bug or not.


List<String> getQueriesAsString();

void addQuery(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.

Perhaps addQueryString to match the above method?

@@ -1,5 +1,15 @@
package sqlancer;

import java.util.List;

public interface Reproducer<G extends GlobalState<?, ?, ?>> {
boolean bugStillTriggers(G globalState);
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps now, we can rename this method to bugTriggers, given that we create the reproducer also if no bug was triggered?

@@ -90,11 +116,12 @@ public void check() throws SQLException {
throw new IgnoreMeException();
}
if (optimizedCount != unoptimizedCount) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could simplify the code by adding reproducer = new SQLite3NoRECReproducer(optimizedQuery, unoptimizedQuery, Arrayas.asList(optimizedQueryString, unoptimizedQueryString)); above the if check and removing the other parts.

Copy link
Contributor

@mrigger mrigger left a comment

Choose a reason for hiding this comment

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

LGMT, thanks!

@mrigger mrigger enabled auto-merge (squash) December 17, 2022 08:47
@mrigger mrigger merged commit b5eb215 into sqlancer:master Dec 17, 2022
albertZhangTJ pushed a commit to albertZhangTJ/sqlancer-lancerfuzz that referenced this pull request Dec 19, 2022
Co-authored-by: Manuel Rigger <rigger@nus.edu.sg>
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.

Support QPG
2 participants