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

Sql fix #314

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Sql fix #314

wants to merge 16 commits into from

Conversation

clackner-gpa
Copy link
Member

No description provided.

Comment on lines +97 to 99
"MeasurementReportingInterval, ConnectionString, Enabled) VALUES({7}, {0}, {1}, {2}, {3}, {4}, {5}, {6})",
"acronym", "name", "protocolID", "framesPerSecond", "measurementReportingInterval",
"connectionString", "enabled");
Copy link
Member

Choose a reason for hiding this comment

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

You need to also include a name for parameter 7. Probably, you should add 7 at the end and insert the name before acronym like so...

Suggested change
"MeasurementReportingInterval, ConnectionString, Enabled) VALUES({7}, {0}, {1}, {2}, {3}, {4}, {5}, {6})",
"acronym", "name", "protocolID", "framesPerSecond", "measurementReportingInterval",
"connectionString", "enabled");
"MeasurementReportingInterval, ConnectionString, Enabled) VALUES({0}, {1}, {2}, {3}, {4}, {5}, {6}, {7})",
"nodeID", "acronym", "name", "protocolID", "framesPerSecond", "measurementReportingInterval",
"connectionString", "enabled");

"acronym", "name", "protocolID", "framesPerSecond", "measurementReportingInterval",
"connectionString", "enabled");

// Insert new device record
connection.ExecuteNonQuery(parameterizedQuery, acronym, name, protocolID, sourceWave.SampleRate, 1000000, $"wavFileName={FilePath.GetAbsolutePath(sourceFileName)}; connectOnDemand=true; outputSourceIDs={acronym}; memoryCache={useMemoryCache}", database.Bool(true));
connection.ExecuteNonQuery(parameterizedQuery, acronym, name, protocolID, sourceWave.SampleRate, 1000000, $"wavFileName={FilePath.GetAbsolutePath(sourceFileName)}; connectOnDemand=true; outputSourceIDs={acronym}; memoryCache={useMemoryCache}", database.Bool(true), nodeID);
Copy link
Member

Choose a reason for hiding this comment

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

This old API for parameterized queries isn't as clever about matching arguments to your format string. Even if you use {7} as the first parameter in the query string, nodeID has to be the first argument to ExecuteNonQuery(). Also, you need database.Guid() or else it won't work for SQLite, PostgreSQL, or Oracle.

Suggested change
connection.ExecuteNonQuery(parameterizedQuery, acronym, name, protocolID, sourceWave.SampleRate, 1000000, $"wavFileName={FilePath.GetAbsolutePath(sourceFileName)}; connectOnDemand=true; outputSourceIDs={acronym}; memoryCache={useMemoryCache}", database.Bool(true), nodeID);
connection.ExecuteNonQuery(parameterizedQuery, database.Guid(nodeID), acronym, name, protocolID, sourceWave.SampleRate, 1000000, $"wavFileName={FilePath.GetAbsolutePath(sourceFileName)}; connectOnDemand=true; outputSourceIDs={acronym}; memoryCache={useMemoryCache}", database.Bool(true));

Comment on lines +180 to +182
command.Parameters.Add(measurement.ID);
command.Parameters.Add((long)measurement.Timestamp);
command.Parameters.Add(measurement.AdjustedValue);
Copy link
Member

Choose a reason for hiding this comment

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

I have a feeling this won't work. IDbCommand.Parameters returns a collection of type IDataParameterCollection which implements the non-generic IList interface. It's entirely possible that the MySQL implementation of the Add() method can convert any old value to an instance of IDbDataParameter, but I feel it's pretty unlikely. You'll need to use command.CreateParameter() and then set the ParameterName and Value fields for each parameter.

@@ -330,7 +330,7 @@ public override void Augment(DataSet configuration)

try
{
database.Connection.ExecuteNonQuery($"DELETE FROM TrackedChange WHERE ID <= {latestVersion}");
database.Connection.ExecuteNonQuery("DELETE FROM TrackedChange WHERE ID <= {0}",latestVersion);
Copy link
Member

Choose a reason for hiding this comment

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

nit, whitespace

Suggested change
database.Connection.ExecuteNonQuery("DELETE FROM TrackedChange WHERE ID <= {0}",latestVersion);
database.Connection.ExecuteNonQuery("DELETE FROM TrackedChange WHERE ID <= {0}", latestVersion);

commandString.Append(')');

command.CommandText = commandString.ToString();
command.CommandText = "INSERT INTO Measurement VALUES ({0}, {1}, {2})";
Copy link
Member

Choose a reason for hiding this comment

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

I missed this. Format strings only work with AdoDataConnection. Otherwise, you need to use the SQL syntax for parameters. In MySQL, that'd be @parameterName.

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