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.

Sign up for GitHub

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

Jump to bottom

Sql fix #314

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

Sql fix #314

wants to merge 34 commits into from

Conversation

clackner-gpa
Copy link
Member

No description provided.

clackner-gpa
clackner-gpa commented Jan 23, 2025
View reviewed changes
Source/Libraries/Adapters/GrafanaAdapters/DeviceAlarmStateAdapter.cs Outdated
if (TargetParentDevices)
newDevices = connection.RetrieveData("SELECT * FROM Device WHERE (IsConcentrator != 0 OR ParentID IS NULL) " +
"AND ID NOT IN (SELECT DeviceID FROM AlarmDevice)").Select();
else newDevices = connection.RetrieveData("SELECT * FROM Device WHERE IsConcentrator = 0 " +
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
else newDevices = connection.RetrieveData("SELECT * FROM Device WHERE IsConcentrator = 0 " +
else
newDevices = connection.RetrieveData("SELECT * FROM Device WHERE IsConcentrator = 0 " +

Copy link

Choose a reason for hiding this comment

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

updated

Source/Libraries/Adapters/HistorianAdapters/LocalOutputAdapter.cs Outdated
@@ -939,11 +939,11 @@ private static void OptimizeLocalHistorianSettings(AdoDataConnection database, s
statusMessage("Optimizing settings for local historians...");

// Load the defined local system historians
IEnumerable<DataRow> historians = database.Connection.RetrieveData(database.AdapterType, $"SELECT AdapterName FROM RuntimeHistorian WHERE NodeID = {nodeIDQueryString} AND TypeName = 'HistorianAdapters.LocalOutputAdapter'").AsEnumerable();
IEnumerable<DataRow> readers = database.Connection.RetrieveData(database.AdapterType, $"SELECT * FROM CustomInputAdapter WHERE NodeID = {nodeIDQueryString} AND TypeName = 'HistorianAdapters.LocalInputAdapter'").AsEnumerable();
IEnumerable<DataRow> historians = database.Connection.RetrieveData(database.AdapterType, "SELECT AdapterName FROM RuntimeHistorian WHERE NodeID = {0} AND TypeName = 'HistorianAdapters.LocalOutputAdapter'", nodeIDQueryString).AsEnumerable();
Copy link
Member Author

Choose a reason for hiding this comment

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

nodeIDQueryString is almost definitely a Query so this won't work. We'd need to update/check wherever OptimizeLocalHistorianSettings is used

Copy link

Choose a reason for hiding this comment

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

updated

Source/Libraries/Adapters/MetadataAdapters/MetadataImportAdapter.cs Outdated
@@ -911,7 +911,7 @@ private Action<DataSet> GetSynchronizeMetadataAction()
List<int> sourceIndicies;

if (definedSourceIndicies.TryGetValue(id, out sourceIndicies))
command.ExecuteNonQuery(deletePhasorSql + $" AND SourceIndex NOT IN ({string.Join(",", sourceIndicies)})", MetadataSynchronizationTimeout, id);
command.ExecuteNonQuery(deletePhasorSql + " AND SourceIndex NOT IN ({0})", string.Join(",", sourceIndicies), MetadataSynchronizationTimeout, id);
Copy link
Member Author

Choose a reason for hiding this comment

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

this won't work either because it turns a list of [1,2,3] into a string "1,2,3" where 1 is not in ("1,2,3")

Copy link

Choose a reason for hiding this comment

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

updated

...s/Adapters/PowerCalculations/PowerMultiCalculator/PowerCalculationConfigurationValidation.cs Outdated
@@ -56,29 +56,32 @@ public static void ValidateDatabaseDefinitions()
/// <param name="database">Database connection to use for checking the data operation</param>
/// <returns>True or false indicating whether the operation exists</returns>
private static bool DataOperationExists(AdoDataConnection database) =>
Convert.ToInt32(database.ExecuteScalar($"SELECT COUNT(*) FROM DataOperation WHERE TypeName='{typeof(PowerCalculationConfigurationValidation).FullName}' AND MethodName='ValidatePowerCalculationConfigurations'")) > 0;
Convert.ToInt32(database.ExecuteScalar("SELECT COUNT(*) FROM DataOperation WHERE TypeName='{0}' AND MethodName='ValidatePowerCalculationConfigurations'", typeof(PowerCalculationConfigurationValidation).FullName)) > 0;
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
Convert.ToInt32(database.ExecuteScalar("SELECT COUNT(*) FROM DataOperation WHERE TypeName='{0}' AND MethodName='ValidatePowerCalculationConfigurations'", typeof(PowerCalculationConfigurationValidation).FullName)) > 0;
Convert.ToInt32(database.ExecuteScalar("SELECT COUNT(*) FROM DataOperation WHERE TypeName = {0} AND MethodName='ValidatePowerCalculationConfigurations'", typeof(PowerCalculationConfigurationValidation).FullName)) > 0;

Copy link

Choose a reason for hiding this comment

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

updated

...s/Adapters/PowerCalculations/PowerMultiCalculator/PowerCalculationConfigurationValidation.cs Outdated
Comment on lines 66 to 67
database.ExecuteNonQuery("INSERT INTO DataOperation(Description, AssemblyName, TypeName, MethodName, Enabled) " +
"VALUES ('Power Calculation Validations', 'PowerCalculations.dll', '{0}', 'ValidatePowerCalculationConfigurations', 1)", typeof(PowerCalculationConfigurationValidation).FullName);
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
database.ExecuteNonQuery("INSERT INTO DataOperation(Description, AssemblyName, TypeName, MethodName, Enabled) " +
"VALUES ('Power Calculation Validations', 'PowerCalculations.dll', '{0}', 'ValidatePowerCalculationConfigurations', 1)", typeof(PowerCalculationConfigurationValidation).FullName);
database.ExecuteNonQuery("INSERT INTO DataOperation(Description, AssemblyName, TypeName, MethodName, Enabled) " +
"VALUES ('Power Calculation Validations', 'PowerCalculations.dll', {0}, 'ValidatePowerCalculationConfigurations', 1)", typeof(PowerCalculationConfigurationValidation).FullName);

Copy link

Choose a reason for hiding this comment

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

updated

Source/Libraries/GSF.Core/Data/DataInserter.cs Outdated
@@ -453,22 +451,23 @@ private void ResetAutoIncValues(Table table)
{
case DatabaseType.SQLServer:
resetAutoIncValueSQL = "DBCC CHECKIDENT('" + table.SQLEscapedName + "', RESEED)";
table.Connection.ExecuteNonQuery(resetAutoIncValueSQL, Timeout);
table.Connection.ExecuteNonQuery("DBCC CHECKIDENT('{0}', RESEED)", table.SQLEscapedName, Timeout);
Copy link
Member Author

Choose a reason for hiding this comment

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

Parameter substitution does not work on Tables

Copy link

Choose a reason for hiding this comment

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

updated

Source/Libraries/GSF.Core/Data/DataInserter.cs Outdated
break;
case DatabaseType.MySQL:
resetAutoIncValueSQL = "ALTER TABLE " + table.SQLEscapedName + " AUTO_INCREMENT = 1";
table.Connection.ExecuteNonQuery(resetAutoIncValueSQL, Timeout);
table.Connection.ExecuteNonQuery("ALTER TABLE {0} AUTO_INCREMENT = 1", table.SQLEscapedName, Timeout);
Copy link
Member Author

Choose a reason for hiding this comment

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

Parameter substitution does not work on Tables

Source/Libraries/GSF.Core/Data/DataInserter.cs Outdated
break;
case DatabaseType.SQLite:
resetAutoIncValueSQL = "DELETE FROM sqlite_sequence WHERE name = '" + table.Name + "'";
table.Connection.ExecuteNonQuery(resetAutoIncValueSQL, Timeout);
table.Connection.ExecuteNonQuery("DELETE FROM sqlite_sequence WHERE name = '{0}'", table.Name, Timeout);
Copy link
Member Author

Choose a reason for hiding this comment

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

Parameter substitution does not work on Tables

Copy link

Choose a reason for hiding this comment

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

updated

Source/Libraries/GSF.Core/Data/DataInserter.cs Outdated
break;
case DatabaseType.PostgreSQL:
// The escaping of names here is very deliberate; for certain table names,
// it is necessary to escape the table name in the pg_get_serial_sequence() call,
// but the call will fail if you attempt to escape the autoIncField name
resetAutoIncValueSQL = $"SELECT setval(pg_get_serial_sequence('{table.SQLEscapedName}', '{table.AutoIncField.Name.ToLower()}'), (SELECT MAX({table.AutoIncField.SQLEscapedName}) FROM {table.SQLEscapedName}))";
table.Connection.ExecuteNonQuery(resetAutoIncValueSQL, Timeout);
table.Connection.ExecuteNonQuery("SELECT setval(pg_get_serial_sequence('{0}', '{1}'), (SELECT MAX({2}) FROM {3}))",
Copy link
Member Author

Choose a reason for hiding this comment

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

Parameter substitution does not work on Tables

Copy link

Choose a reason for hiding this comment

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

updated

Source/Libraries/GSF.Core/Data/DataInserter.cs Outdated
@@ -616,7 +615,7 @@ private void ExecuteInserts(Table fromTable, Table toTable)
case DatabaseType.SQLServer:
try
{
toTable.Connection.ExecuteNonQuery("SET IDENTITY_INSERT " + toTable.SQLEscapedName + " ON", Timeout);
toTable.Connection.ExecuteNonQuery("SET IDENTITY_INSERT {0} ON", toTable.SQLEscapedName, Timeout);
Copy link
Member Author

Choose a reason for hiding this comment

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

Parameter substitution does not work on Tables

Copy link

Choose a reason for hiding this comment

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

updated

@collins-self collins-self force-pushed the SQL_Fix branch 2 times, most recently from dc77206 to ca4ad88 Compare February 7, 2025 16:01
@StephenCWills
Copy link
Member

StephenCWills commented Feb 8, 2025 •
edited
Loading

collins-self force-pushed the SQL_Fix branch from 95e045f to 9d926ff last week

FYI, you force-pushed the 95e045f commit out of existence. That reverted the fixes to the issues I brought up initially so I'm unresolving them.

@collins-self
Copy link

collins-self commented Feb 10, 2025 •
edited
Loading

collins-self force-pushed the SQL_Fix branch from 95e045f to 9d926ff last week

FYI, you force-pushed the 95e045f commit out of existence. That reverted the fixes to the issues I brought up initially so I'm unresolving them.

@StephenCWills
That's my bad, I added the changes from that commit in the most recent commit.

StephenCWills
StephenCWills requested changes Feb 10, 2025
View reviewed changes
Source/Applications/Wave Demo Apps/UpdateWAVMetaData/Program.cs Outdated

// 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), database.Guid(nodeID)); 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 doesn't work, because NodeID is the first column in the INSERT list. If you absolutely must put these fields at the end of the parameters list, then you need to also modify the INSERT list to put NodeID at the end. However, I think it would make more sense to go with my original suggestion and just put the "nodeID" and database.Guid(nodeID) parameters at the beginning of their respective parameter lists.

Copy link
Member

Choose a reason for hiding this comment

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

GitHub is obnoxious. In case it helps, know that I had selected lines 96-102 before making this comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@StephenCWills StephenCWills StephenCWills requested changes

@collins-self collins-self collins-self left review comments

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@clackner-gpa @StephenCWills @collins-self