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

New vs. delete mismatch #12142

Closed
1 of 2 tasks
krlmlr opened this issue May 20, 2024 · 10 comments
Closed
1 of 2 tasks

New vs. delete mismatch #12142

krlmlr opened this issue May 20, 2024 · 10 comments

Comments

@krlmlr
Copy link
Collaborator

krlmlr commented May 20, 2024

What happens?

clang-asan detected a peculiar error: https://github.com/duckdb/duckdb-r/actions/runs/9162129815/job/25188489919#step:6:249 .

To Reproduce

Run tests with clang-asan? The location might be obvious from the logs too.

OS:

Linux with clang-asan

DuckDB Version:

5719307

DuckDB Client:

R

Full Name:

Kirill Müller

Affiliation:

cynkra GmbH

What is the latest build you tested with? If possible, we recommend testing with the latest nightly build.

I have not tested with any build

Did you include all relevant data sets for reproducing the issue?

Not applicable - the reproduction does not require a data set

Did you include all code required to reproduce the issue?

  • Yes, I have

Did you include all relevant configuration (e.g., CPU architecture, Python version, Linux distribution) to reproduce the issue?

  • Yes, I have
@carlopi
Copy link
Contributor

carlopi commented May 20, 2024

It's not completely obvious to me where this is coming from, how to reproduce, could you possibly share some more info?

It's debug or release, what set of flags, and it's by any chance DEBUG_STACKTRACE enabled?

@krlmlr
Copy link
Collaborator Author

krlmlr commented May 21, 2024

I'm rerunning this job. If this is green, we might have a Heisenbug. If not, I'll dig a little deeper.

@Tishj
Copy link
Contributor

Tishj commented May 21, 2024

I think we'd just appreciate context, regardless if this is reproducable or not, now we can't even reason about it

@carlopi
Copy link
Contributor

carlopi commented May 21, 2024

Given it's a runtime check it might not be conclusive, since it can be non-deterministic, and on the other side it could potentially be a false positive / or something weird going on with toolchain, but either way available info or, bonus points, a reproduction would be handy.

I tried to do some checks / review the code, but haven't found anything obvious.

@Tishj
Copy link
Contributor

Tishj commented May 21, 2024

By the looks of it, the r-api tried to do a prepare, that prepare hit an exception

		if (result->return_type.id() == LogicalTypeId::UNKNOWN) {
			throw ParameterNotResolvedException();
		}

I assume that exception is not being handled before returning to R context, which causes the problem?

	auto stmt = conn->conn->Prepare(std::move(statements.back()));
	if (stmt->HasError()) {
		cpp11::stop("rapi_prepare: Failed to prepare query %s\nError: %s", query.c_str(),
		            stmt->error.Message().c_str());
	}

this might need a try-catch block on the R repos side?

@Tishj
Copy link
Contributor

Tishj commented May 21, 2024

Though in the C-API we also don't put a try-catch around this:

duckdb_state duckdb_prepare_extracted_statement(duckdb_connection connection,
                                                duckdb_extracted_statements extracted_statements, idx_t index,
                                                duckdb_prepared_statement *out_prepared_statement) {
	Connection *conn = reinterpret_cast<Connection *>(connection);
	auto source_wrapper = (ExtractStatementsWrapper *)extracted_statements;

	if (!connection || !out_prepared_statement || index >= source_wrapper->statements.size()) {
		return DuckDBError;
	}
	auto wrapper = new PreparedStatementWrapper();
	wrapper->statement = conn->Prepare(std::move(source_wrapper->statements[index]));

	*out_prepared_statement = (duckdb_prepared_statement)wrapper;
	return wrapper->statement->HasError() ? DuckDBError : DuckDBSuccess;
}

@krlmlr
Copy link
Collaborator Author

krlmlr commented May 21, 2024

@carlopi
Copy link
Contributor

carlopi commented May 27, 2024

@krlmlr: do you happened to have this reproduced or any more information?

Trying to understand if there is anything needed duckdb-side on this

@krlmlr
Copy link
Collaborator Author

krlmlr commented May 27, 2024

On it.

@krlmlr
Copy link
Collaborator Author

krlmlr commented Jun 30, 2024

Could be that this error originates from the glue code because I'm also seeing a similar version of the error with code that doesn't descend into the C++ library at all. I will reopen or open a new issue with more detail if needed.

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

No branches or pull requests

5 participants