Skip to content

Conversation

@CurtHagenlocher
Copy link
Contributor

The vectorized scanner seems to be causing performance issues under some circumstances. Add an option to disable it.

@github-actions github-actions bot added this to the ADBC Libraries 21 milestone Oct 10, 2025
OptionStatementIngestTargetFileSize = "adbc.snowflake.statement.ingest_target_file_size"
OptionStatementIngestCompressionCodec = "adbc.snowflake.statement.ingest_compression_codec" // TODO(GH-1473): Implement option
OptionStatementIngestCompressionLevel = "adbc.snowflake.statement.ingest_compression_level" // TODO(GH-1473): Implement option
OptionStatementVectorizedScanner = "adbc.snowflake.statement.use_vectorized_scanner"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm... should probably reference ingestion. Maybe ingest_use_vectorized_scanner?

Copy link
Member

Choose a reason for hiding this comment

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

Sounds reasonable to me.

(Also not quite an issue for here but the adbc. prefix is probably redundant)

})

// Create a temporary stage, we can't start uploading until it has been created
createTemporaryStageStmt := fmt.Sprintf(createTemporaryStageTmpl, "")
Copy link
Member

Choose a reason for hiding this comment

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

Why don't we handle the option here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My read of the code is that this only ever binds a single record so I don't know why you'd ever need the vectorized scanner.

Copy link
Member

Choose a reason for hiding this comment

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

It could bind a single very large record :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this for consistency and compatibility, though I'm skeptical that there's any real-world value ;).

@CurtHagenlocher CurtHagenlocher merged commit 1f8b159 into apache:main Oct 13, 2025
42 checks passed
@CurtHagenlocher CurtHagenlocher deleted the VectorizedScanner branch October 17, 2025 20:08
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.

3 participants