-
Notifications
You must be signed in to change notification settings - Fork 97
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
feat(csharp/src/Drivers/Apache): add implementation for AdbcStatement.SetOption on Spark driver #1849
feat(csharp/src/Drivers/Apache): add implementation for AdbcStatement.SetOption on Spark driver #1849
Conversation
|
||
protected internal int BatchSize { get; private set; } = BatchSizeDefault; | ||
|
||
public static class StatementOptions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making this public
seems like the best choice. But if you think it should be private, let me know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or maybe made available at the SparkStatement
level?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we establish a convention of having a static class in appropriate places, like
class HiveServer2Statement : AdbcStatement {
public static class Options {
// options common to all HiveServer2-derived drivers go here
public static string PollTimeMilliseconds = "...";
}
}
class SparkStatement : HiveServer2Statement {
new public static class Options : HiveServer2Statement.Options {
// options specific to Spark go here
// ...
}
}
|
||
protected internal int BatchSize { get; private set; } = BatchSizeDefault; | ||
|
||
public static class StatementOptions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we establish a convention of having a static class in appropriate places, like
class HiveServer2Statement : AdbcStatement {
public static class Options {
// options common to all HiveServer2-derived drivers go here
public static string PollTimeMilliseconds = "...";
}
}
class SparkStatement : HiveServer2Statement {
new public static class Options : HiveServer2Statement.Options {
// options specific to Spark go here
// ...
}
}
} | ||
else | ||
{ | ||
statement.SetOption(SparkStatement.Options.BatchSize, value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these tests roundtrip and verify the option is set correctly?
Implement AdbcStatement.SetOption on Spark driver
"adbc.statement.polltime_milliseconds"
-> sets the poll time to check for results to execute a statement."adbc.statement.batch_size"
-> sets the maximum size of a single batch to receive.