-
Notifications
You must be signed in to change notification settings - Fork 148
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
Feature-level parameter for defaults (ColumnOptions constructor) #164
Comments
The level It seems that you want to do two things with the same enumeration. You want to provide a way to specify a group of default values, as well as providing a way to validate the configuration for a specific version of SQL Server. |
Great feedback. I agree
So I would instead suggest the enum only reflects SQL versions, plus Regarding other enum values, I'm proposing a given SQL level merely reflects a minimally optimized variation on the So it was probably wrong of me to say configuration validation was a less-important role for this idea. It's actually the primary role, and mapping the defaults is more of a side-effect of declaring the SQL version. (And speaking of CCI, if we accept validation as an important role, we'd probably also want |
Bit of a late reply... I'm not sure if I would view the providing of defaults as an aspect of (recognizing) available features. The available features determine what set of configurations are valid. Among those, one will be the most suitable default (in general). Another may be better suited to a specific use case. The name Legacy sounds good. If it said The best argument I can think of for having multiple default configurations is that it would make people select one of the defaults, and that should be a good configuration depending on their specifics. If you have to make your own configuration from scratch, it's easier to make a mistake. The smaller the selection, the easier to make the optimal choice. |
As time goes on, SQL server releases add or remove certain features. Similarly, this sink has many defaults that exist purely for backwards-compatibility reasons, despite learning later that other defaults would be better. For example, the
Level
Standard Column defaults to 128 bytes when 12 would be adequate (#146), or the use of aDateTime
column forTimeStamp
rather than the superior SQL 2008DateTimeOffset
type.The PR I'm working on consolidates most of these defaults into the
ColumnOptions
constructor, and I'm considering an override (not in the same PR) that accepts an enumeration which maps to groups of default values.The most important aspect of this, in my opinion, would be
OptimizedDefaults
which are performance-oriented without getting into changes that require a specific version of SQL Server:Id
column (see benchmarks)Level
to 12 characters instead of 128LogEvent
Standard Column by default instead ofProperties
DisableTriggers
totrue
by defaultWhile of less importance, this could also help inform configuration validation which could reduce support questions. For example, clustered columnstore indexing only supports
max
length character data with SQL 2017 or later. If this index type was chosen but the feature-level didn't map to SQL 2017, an exception could be thrown. When SQL 2017 is selected themax
length would be allowed.Perhaps we can start a discussion about how these settings would impact the defaults, other possible levels, or whether this is just unnecessary noise (I'm thick-skinned, it's fine). I'm even strongly tempted to suggest a breaking change by making
OptimizedDefaults
the default (the default defaults?) and anyone who is truly tied to backwards-compatibility can explicitly specify that level if necessary.Ideas?
The text was updated successfully, but these errors were encountered: