Repository: NREL/OpenStudio (language: python)

Issue:
Glass U Factor sql error
Issue overview
--------------
User on Unmet Hours hits an error when using `window.uFactor`. Reported at https://unmethours.com/question/80413/problem-in-openstudio-measure-in-34/.

Looks like a closing single quotation mark is missing from all these sql statements:
https://github.com/NREL/OpenStudio/blob/135d27832ebee5682c545a2a6f507e25e0cffa40/src/model/SubSurface.cpp#L377
https://github.com/NREL/OpenStudio/blob/135d27832ebee5682c545a2a6f507e25e0cffa40/src/model/SubSurface.cpp#L450
https://github.com/NREL/OpenStudio/blob/d75af8b9309517294089066e6a5b3c0373d43d9e/src/model/Surface.cpp#L659
https://github.com/NREL/OpenStudio/blob/d75af8b9309517294089066e6a5b3c0373d43d9e/src/model/Surface.cpp#L732



Discussion / hints:
This looks like my fault in https://github.com/NREL/OpenStudio/pull/4550. @jmarrec is https://github.com/NREL/OpenStudio/tree/develop/src/utilities/sql/Test an appropriate place to add tests?
Looks to me like https://github.com/NREL/OpenStudio/commit/e91017d1813564dc7d1a42ece8a841f9c04702cb is to blame.

Fix patch:
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 1cf0c54da69..3c89f039454 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -6,7 +6,7 @@ set(CMAKE_CXX_STANDARD_REQUIRED ON)
 # Do not enable compiler specific extensions, for eg on GCC use -std=c++1z (=c++17) and not -std=gnu++17
 set(CMAKE_CXX_EXTENSIONS OFF)
 
-# Use ccache if available, has to be before "project()"
+# Use ccache is available, has to be before "project()"
 find_program(CCACHE_PROGRAM ccache)
 if(CCACHE_PROGRAM)
   # Support Unix Makefiles and Ninja
diff --git a/src/model/SubSurface.cpp b/src/model/SubSurface.cpp
index 145ac6dcae9..5071dce4277 100644
--- a/src/model/SubSurface.cpp
+++ b/src/model/SubSurface.cpp
@@ -338,48 +338,30 @@ namespace model {
         OptionalSqlFile sqlFile = model().sqlFile();
         OptionalString constructionName = oConstruction->name();
         OptionalDouble outputResult;
+        OptionalString subSurfaceName = name();
+
         // opaque exterior
         if (sqlFile && constructionName && oConstruction->isOpaque()) {
-          std::string query = R"(SELECT RowId from TabularDataWithStrings
-                                      WHERE ReportName = 'EnvelopeSummary'
-                                        AND ReportForString = 'Entire Facility'
-                                        AND TableName = 'Opaque Exterior'
-                                        AND ColumnName = 'Construction'
-                                        AND Value = ?;)";
-          OptionalInt rowId = sqlFile->execAndReturnFirstInt(query, to_upper_copy(*constructionName));
-
-          if (rowId) {
-            std::string query = R"(SELECT Value from TabularDataWithStrings
-                                      WHERE ReportName = 'EnvelopeSummary'
-                                        AND ReportForString = 'Entire Facility'
-                                        AND TableName = 'Opaque Exterior'
-                                        AND ColumnName = 'U-Factor with Film'
-                                        AND Units='W/m2-K'
-                                        AND RowId = ?;)";
-            outputResult = sqlFile->execAndReturnFirstDouble(query, *rowId);
-          }
+          std::string query = R"(SELECT Value from TabularDataWithStrings
+                                    WHERE ReportName = 'EnvelopeSummary'
+                                      AND ReportForString = 'Entire Facility'
+                                      AND TableName = 'Opaque Exterior'
+                                      AND ColumnName = 'U-Factor with Film'
+                                      AND Units='W/m2-K'
+                                      AND RowName = ?;)";
+          outputResult = sqlFile->execAndReturnFirstDouble(query, to_upper_copy(*subSurfaceName));
         }
 
         // fenestration
         if (sqlFile && constructionName && oConstruction->isFenestration()) {
-          std::string query = R"(SELECT RowId from TabularDataWithStrings
-                                      WHERE ReportName = 'EnvelopeSummary'
-                                        AND ReportForString = 'Entire Facility'
-                                        AND TableName = 'Exterior Fenestration'
-                                        AND ColumnName = 'Construction'
-                                        AND Value = ?;)";
-          OptionalInt rowId = sqlFile->execAndReturnFirstInt(query, to_upper_copy(*constructionName));
-
-          if (rowId) {
-            std::string query = R"(SELECT Value from TabularDataWithStrings
-                                      WHERE ReportName = 'EnvelopeSummary'
-                                        AND ReportForString = 'Entire Facility'
-                                        AND TableName = 'Exterior Fenestration
-                                        AND ColumnName = 'Glass U-Factor'
-                                        AND Units='W/m2-K'
-                                        AND RowId = ?;)";
-            outputResult = sqlFile->execAndReturnFirstDouble(query, *rowId);
-          }
+          std::string query = R"(SELECT Value from TabularDataWithStrings
+                                    WHERE ReportName = 'EnvelopeSummary'
+                                      AND ReportForString = 'Entire Facility'
+                                      AND TableName = 'Exterior Fenestration'
+                                      AND ColumnName = 'Glass U-Factor'
+                                      AND Units='W/m2-K'
+                                      AND RowName = ?;)";
+          outputResult = sqlFile->execAndReturnFirstDouble(query, to_upper_copy(*subSurfaceName));
         }
 
         if (inputResult) {
@@ -408,51 +390,32 @@ namespace model {
         OptionalSqlFile sqlFile = model().sqlFile();
         OptionalString constructionName = oConstruction->name();
         OptionalDouble outputResult;
+        OptionalString subSurfaceName = name();
+
         // opaque exterior
         if (sqlFile && constructionName && oConstruction->isOpaque()) {
-          std::string query = R"(SELECT RowId from TabularDataWithStrings
-                                      WHERE ReportName = 'EnvelopeSummary'
-                                        AND ReportForString = 'Entire Facility'
-                                        AND TableName = 'Opaque Exterior'
-                                        AND ColumnName = 'Construction'
-                                        AND Value = ?;)";
-          OptionalInt rowId = sqlFile->execAndReturnFirstInt(query, to_upper_copy(*constructionName));
-
-          if (rowId) {
-            std::string query = R"(SELECT Value from TabularDataWithStrings
-                                      WHERE ReportName = 'EnvelopeSummary'
-                                        AND ReportForString = 'Entire Facility'
-                                        AND TableName = 'Opaque Exterior'
-                                        AND ColumnName = 'U-Factor no Film'
-                                        AND Units='W/m2-K'
-                                        AND RowId = ?;)";
-            outputResult = sqlFile->execAndReturnFirstDouble(query, *rowId);
-          }
+          std::string query = R"(SELECT Value from TabularDataWithStrings
+                                    WHERE ReportName = 'EnvelopeSummary'
+                                      AND ReportForString = 'Entire Facility'
+                                      AND TableName = 'Opaque Exterior'
+                                      AND ColumnName = 'U-Factor no Film'
+                                      AND Units='W/m2-K'
+                                      AND RowName = ?;)";
+          outputResult = sqlFile->execAndReturnFirstDouble(query, to_upper_copy(*subSurfaceName));
         }
 
         // fenestration
         if (sqlFile && constructionName && oConstruction->isFenestration()) {
 
           // get u-factor, then subtract film coefficients
-
-          std::string query = R"(SELECT RowId from TabularDataWithStrings
-                                      WHERE ReportName = 'EnvelopeSummary'
-                                        AND ReportForString = 'Entire Facility'
-                                        AND TableName = 'Exterior Fenestration'
-                                        AND ColumnName = 'Construction'
-                                        AND Value = ?;)";
-          OptionalInt rowId = sqlFile->execAndReturnFirstInt(query, to_upper_copy(*constructionName));
-
-          if (rowId) {
-            std::string query = R"(SELECT Value from TabularDataWithStrings
-                                      WHERE ReportName = 'EnvelopeSummary'
-                                        AND ReportForString = 'Entire Facility'
-                                        AND TableName = 'Exterior Fenestration
-                                        AND ColumnName = 'Glass U-Factor'
-                                        AND Units='W/m2-K'
-                                        AND RowId = ?;)";
-            outputResult = sqlFile->execAndReturnFirstDouble(query, *rowId);
-          }
+          std::string query = R"(SELECT Value from TabularDataWithStrings
+                                    WHERE ReportName = 'EnvelopeSummary'
+                                      AND ReportForString = 'Entire Facility'
+                                      AND TableName = 'Exterior Fenestration'
+                                      AND ColumnName = 'Glass U-Factor'
+                                      AND Units='W/m2-K'
+                                      AND RowName = ?;)";
+          outputResult = sqlFile->execAndReturnFirstDouble(query, to_upper_copy(*subSurfaceName));
 
           if (outputResult) {
             outputResult = 1.0 / (1.0 / (*outputResult) - oSurface->filmResistance());
diff --git a/src/model/Surface.cpp b/src/model/Surface.cpp
index 58291b4dbef..8d306aa0483 100644
--- a/src/model/Surface.cpp
+++ b/src/model/Surface.cpp
@@ -620,48 +620,30 @@ namespace model {
         OptionalSqlFile sqlFile = model().sqlFile();
         OptionalString constructionName = oConstruction->name();
         OptionalDouble outputResult;
+        OptionalString surfaceName = name();
+
         // opaque exterior
         if (sqlFile && constructionName && oConstruction->isOpaque()) {
-          std::string query = R"(SELECT RowId from TabularDataWithStrings
-                                      WHERE ReportName = 'EnvelopeSummary'
-                                        AND ReportForString = 'Entire Facility'
-                                        AND TableName = 'Opaque Exterior'
-                                        AND ColumnName = 'Construction'
-                                        AND Value = ?;)";
-          OptionalInt rowId = sqlFile->execAndReturnFirstInt(query, to_upper_copy(*constructionName));
-
-          if (rowId) {
-            std::string query = R"(SELECT Value from TabularDataWithStrings
-                                      WHERE ReportName = 'EnvelopeSummary'
-                                        AND ReportForString = 'Entire Facility'
-                                        AND TableName = 'Opaque Exterior'
-                                        AND ColumnName = 'U-Factor with Film'
-                                        AND Units='W/m2-K'
-                                        AND RowId = ?;)";
-            outputResult = sqlFile->execAndReturnFirstDouble(query, *rowId);
-          }
+          std::string query = R"(SELECT Value from TabularDataWithStrings
+                                    WHERE ReportName = 'EnvelopeSummary'
+                                      AND ReportForString = 'Entire Facility'
+                                      AND TableName = 'Opaque Exterior'
+                                      AND ColumnName = 'U-Factor with Film'
+                                      AND Units='W/m2-K'
+                                      AND RowName = ?;)";
+          outputResult = sqlFile->execAndReturnFirstDouble(query, to_upper_copy(*surfaceName));
         }
 
         // fenestration
         if (sqlFile && constructionName && oConstruction->isFenestration()) {
-          std::string query = R"(SELECT RowId from TabularDataWithStrings
-                                      WHERE ReportName = 'EnvelopeSummary'
-                                        AND ReportForString = 'Entire Facility'
-                                        AND TableName = 'Exterior Fenestration'
-                                        AND ColumnName = 'Construction'
-                                        AND Value = ?;)";
-          OptionalInt rowId = sqlFile->execAndReturnFirstInt(query, to_upper_copy(*constructionName));
-
-          if (rowId) {
-            std::string query = R"(SELECT Value from TabularDataWithStrings
-                                      WHERE ReportName = 'EnvelopeSummary'
-                                        AND ReportForString = 'Entire Facility'
-                                        AND TableName = 'Exterior Fenestration
-                                        AND ColumnName = 'Glass U-Factor'
-                                        AND Units='W/m2-K'
-                                        AND RowId = ?;)";
-            outputResult = sqlFile->execAndReturnFirstDouble(query, *rowId);
-          }
+          std::string query = R"(SELECT Value from TabularDataWithStrings
+                                    WHERE ReportName = 'EnvelopeSummary'
+                                      AND ReportForString = 'Entire Facility'
+                                      AND TableName = 'Exterior Fenestration'
+                                      AND ColumnName = 'Glass U-Factor'
+                                      AND Units='W/m2-K'
+                                      AND RowName = ?;)";
+          outputResult = sqlFile->execAndReturnFirstDouble(query, to_upper_copy(*surfaceName));
         }
 
         if (inputResult) {
@@ -689,52 +671,32 @@ namespace model {
         OptionalSqlFile sqlFile = model().sqlFile();
         OptionalString constructionName = oConstruction->name();
         OptionalDouble outputResult;
+        OptionalString surfaceName = name();
+
         // opaque exterior
         if (sqlFile && constructionName && oConstruction->isOpaque()) {
-          std::string query = R"(SELECT RowId from TabularDataWithStrings
-                                      WHERE ReportName = 'EnvelopeSummary'
-                                        AND ReportForString = 'Entire Facility'
-                                        AND TableName = 'Opaque Exterior'
-                                        AND ColumnName = 'Construction'
-                                        AND Value = ?;)";
-          OptionalInt rowId = sqlFile->execAndReturnFirstInt(query, to_upper_copy(*constructionName));
-
-          if (rowId) {
-            std::string query = R"(SELECT Value from TabularDataWithStrings
-                                      WHERE ReportName = 'EnvelopeSummary'
-                                        AND ReportForString = 'Entire Facility'
-                                        AND TableName = 'Opaque Exterior'
-                                        AND ColumnName = 'U-Factor no Film'
-                                        AND Units='W/m2-K'
-                                        AND RowId = ?;)";
-            outputResult = sqlFile->execAndReturnFirstDouble(query, *rowId);
-          }
+          std::string query = R"(SELECT Value from TabularDataWithStrings
+                                    WHERE ReportName = 'EnvelopeSummary'
+                                      AND ReportForString = 'Entire Facility'
+                                      AND TableName = 'Opaque Exterior'
+                                      AND ColumnName = 'U-Factor no Film'
+                                      AND Units='W/m2-K'
+                                      AND RowName = ?;)";
+          outputResult = sqlFile->execAndReturnFirstDouble(query, to_upper_copy(*surfaceName));
         }
 
         // fenestration
         if (sqlFile && constructionName && oConstruction->isFenestration()) {
 
           // get u-factor, then subtract film coefficients
-
-          std::string query = R"(SELECT RowId from TabularDataWithStrings
-                                      WHERE ReportName = 'EnvelopeSummary'
-                                        AND ReportForString = 'Entire Facility'
-                                        AND TableName = 'Exterior Fenestration'
-                                        AND ColumnName = 'Construction'
-                                        AND Value = ?;)";
-          OptionalInt rowId = sqlFile->execAndReturnFirstInt(query, to_upper_copy(*constructionName));
-
-          if (rowId) {
-            // TODO: this is exactly the same as the uFactor one
-            std::string query = R"(SELECT Value from TabularDataWithStrings
-                                      WHERE ReportName = 'EnvelopeSummary'
-                                        AND ReportForString = 'Entire Facility'
-                                        AND TableName = 'Exterior Fenestration
-                                        AND ColumnName = 'Glass U-Factor'
-                                        AND Units='W/m2-K'
-                                        AND RowId = ?;)";
-            outputResult = sqlFile->execAndReturnFirstDouble(query, *rowId);
-          }
+          std::string query = R"(SELECT Value from TabularDataWithStrings
+                                    WHERE ReportName = 'EnvelopeSummary'
+                                      AND ReportForString = 'Entire Facility'
+                                      AND TableName = 'Exterior Fenestration'
+                                      AND ColumnName = 'Glass U-Factor'
+                                      AND Units='W/m2-K'
+                                      AND RowName = ?;)";
+          outputResult = sqlFile->execAndReturnFirstDouble(query, to_upper_copy(*surfaceName));
 
           if (outputResult) {
             outputResult = 1.0 / (1.0 / (*outputResult) - filmResistance());


Test patch:
diff --git a/src/model/test/SubSurface_GTest.cpp b/src/model/test/SubSurface_GTest.cpp
index 91e29401312..fef5e209f00 100644
--- a/src/model/test/SubSurface_GTest.cpp
+++ b/src/model/test/SubSurface_GTest.cpp
@@ -53,6 +53,7 @@
 #include "../ShadingControl.hpp"
 #include "../Model_Impl.hpp"
 #include "../ThermalZone.hpp"
+#include "../PlanarSurface.hpp"
 
 #include "../../energyplus/ReverseTranslator.hpp"
 #include "../../utilities/sql/SqlFile.hpp"
@@ -1565,3 +1566,43 @@ TEST_F(ModelFixture, 4403_FenestrationAssembly) {
   ASSERT_TRUE(subSurface->assemblyVisibleTransmittance());
   EXPECT_EQ(0.440, subSurface->assemblyVisibleTransmittance().get());
 }
+
+TEST_F(ModelFixture, 4678_SubSurfaceGlassUFactorSqlError) {
+  // Test for #4678 - Glass U Factor sql error
+
+  // This one has fenestration that includes WindowProperty:FrameAndDivider
+  openstudio::path idfPath = resourcesPath() / toPath("energyplus/FrameAndDivider/in.idf");
+  energyplus::ReverseTranslator reverseTranslator;
+  ASSERT_NO_THROW(reverseTranslator.loadModel(idfPath));
+  OptionalModel _model = reverseTranslator.loadModel(idfPath);
+  ASSERT_TRUE(_model);
+  Model model = _model.get();
+
+  openstudio::path sqlPath = resourcesPath() / toPath("energyplus/FrameAndDivider/eplusout.sql");
+  openstudio::SqlFile sqlFile = openstudio::SqlFile(sqlPath);
+  ASSERT_TRUE(sqlFile.connectionOpen());
+
+  model.setSqlFile(sqlFile);
+  ASSERT_TRUE(model.sqlFile());
+
+  OptionalSubSurface subSurface = model.getModelObjectByName<SubSurface>("Story 1 Core Space Exterior Wall Window");
+  ASSERT_TRUE(subSurface);
+
+  OptionalConstructionBase oConstruction = subSurface->construction();
+  OptionalSurface oSurface = subSurface->surface();
+
+  ASSERT_TRUE(oConstruction);
+  ASSERT_TRUE(oSurface);
+
+  EXPECT_FALSE(oConstruction->isOpaque());
+  EXPECT_TRUE(oConstruction->isFenestration());
+
+  ASSERT_TRUE(subSurface->uFactor());
+  double uFactor = subSurface->uFactor().get();
+  EXPECT_DOUBLE_EQ(2.559, uFactor);
+
+  double filmResistance = oSurface->filmResistance();
+  double thermalConductance = 1.0 / (1.0 / (uFactor)-filmResistance);
+  ASSERT_TRUE(subSurface->thermalConductance());
+  EXPECT_DOUBLE_EQ(thermalConductance, subSurface->thermalConductance().get());
+}
diff --git a/src/model/test/Surface_GTest.cpp b/src/model/test/Surface_GTest.cpp
index 52683ca48a2..9c7a4ec783d 100644
--- a/src/model/test/Surface_GTest.cpp
+++ b/src/model/test/Surface_GTest.cpp
@@ -75,7 +75,9 @@
 #include "../ShadingSurfaceGroup.hpp"
 #include "../SurfacePropertyOtherSideCoefficients.hpp"
 #include "../SurfacePropertyOtherSideConditionsModel.hpp"
+#include "../PlanarSurface.hpp"
 
+#include "../../energyplus/ReverseTranslator.hpp"
 #include "../../utilities/idf/IdfObject.hpp"
 #include "../../utilities/idf/WorkspaceWatcher.hpp"
 #include "../../utilities/idf/WorkspaceExtensibleGroup.hpp"
@@ -3798,3 +3800,40 @@ TEST_F(ModelFixture, Issue_4374) {
     EXPECT_TRUE(surface.subSurfaces().size() <= 1);
   }
 }
+
+TEST_F(ModelFixture, 4678_SurfaceGlassUFactorSqlError) {
+  // Test for #4678 - Glass U Factor sql error
+
+  // This one has fenestration that includes WindowProperty:FrameAndDivider
+  openstudio::path idfPath = resourcesPath() / toPath("energyplus/FrameAndDivider/in.idf");
+  energyplus::ReverseTranslator reverseTranslator;
+  ASSERT_NO_THROW(reverseTranslator.loadModel(idfPath));
+  OptionalModel _model = reverseTranslator.loadModel(idfPath);
+  ASSERT_TRUE(_model);
+  Model model = _model.get();
+
+  openstudio::path sqlPath = resourcesPath() / toPath("energyplus/FrameAndDivider/eplusout.sql");
+  openstudio::SqlFile sqlFile = openstudio::SqlFile(sqlPath);
+  ASSERT_TRUE(sqlFile.connectionOpen());
+
+  model.setSqlFile(sqlFile);
+  ASSERT_TRUE(model.sqlFile());
+
+  OptionalSurface surface = model.getModelObjectByName<Surface>("Story 1 Core Space Exterior Wall");
+  ASSERT_TRUE(surface);
+
+  OptionalConstructionBase oConstruction = surface->construction();
+
+  ASSERT_TRUE(oConstruction);
+
+  EXPECT_TRUE(oConstruction->isOpaque());
+  EXPECT_FALSE(oConstruction->isFenestration());
+
+  ASSERT_TRUE(surface->uFactor());
+  double uFactor = surface->uFactor().get();
+  EXPECT_NEAR(0.310, uFactor, 1E-03);
+
+  ASSERT_TRUE(surface->thermalConductance());
+  double thermalConductance = surface->thermalConductance().get();
+  EXPECT_NEAR(0.325, thermalConductance, 1E-03);
+}