feat: add Microsoft SQL Server Offline store. #1136
Conversation
|
@sdreyer can you have a look into this ? |
There was a problem hiding this comment.
Thanks @error9098x for the PR! I have left you some comments. You have most of the backend but will need to add a Microsoft SQL Server in order for us to test it. Also, you will need to expose registering a Microsoft SQL to the user by adding the client side of things as well. Let me know if you have any questions.
| query = fmt.Sprintf("CREATE VIEW %s AS SELECT %s as entity, %s as value, CONVERT(datetimeOffset, '%s', 120) as ts FROM %s", | ||
| sanitize(tableName), sanitize(schema.Entity), sanitize(schema.Value), time.Now().UTC().Format("2006-01-02 15:04:05"), sanitize(schema.SourceTable)) | ||
| } | ||
| fmt.Printf("Resource creation query: %s", query) |
There was a problem hiding this comment.
can we remove this print statement?
| return q.trainingSetQuery(store, def, tableName, labelName, true) | ||
| } | ||
|
|
||
| func (q MSSQLQueries) trainingSetQuery(store *sqlOfflineStore, def TrainingSetDef, tableName string, labelName string, isUpdate bool) error { |
There was a problem hiding this comment.
The training set query will need to support lag features as well. Here is an example of lag features for other sql providers.
| } | ||
|
|
||
| func (q MSSQLQueries) numRows(n interface{}) (int64, error) { | ||
| return n.(int64), nil |
There was a problem hiding this comment.
converting this into
numRows, ok := n.(int64)
would be safer in order to avoid panic.
| } | ||
|
|
||
| func (ms MSSQLConfig) MutableFields() ss.StringSet { | ||
| return ss.StringSet{ |
There was a problem hiding this comment.
does it make sense to have everything mutable? It would be essentially a different provider if they change everything.
There was a problem hiding this comment.
I think it makes sense to have the Username and Password be mutable only.
Description
This PR adds Microsoft SQL Server as an Offline Store.
I have tried my best to add all functionalities from the Postgres & MySQL Offline Store.
I have used the MSSQL equivalent of Materialized Views which is Indexed Views. I made sure no function is vulnerable to SQL Injection. I didn't add any extra features in this PR, it's mostly the implementation of all the methods to satisfy the interface definition.
Type of change
Feature: Implemented Microsoft SQL Server as an Offline Store
Does this correspond to an open issue?
Closes: #1117
Select type(s) of change
Checklist: