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

No requirement can be found for test "feature S R Sconsistency" #78

Closed
dstenger opened this issue Jul 3, 2018 · 9 comments · Fixed by #110
Closed

No requirement can be found for test "feature S R Sconsistency" #78

dstenger opened this issue Jul 3, 2018 · 9 comments · Fixed by #110
Assignees
Labels
Projects
Milestone

Comments

@dstenger
Copy link
Contributor

dstenger commented Jul 3, 2018

Test:

/**
* Test case
* /base/core/contents/data/table_def/srs_id
*
* @see <a href="http://www.geopackage.org/spec120/index.html" target=
* "_blank">Contents - Requirement 13</a>
*
* @throws SQLException
* If an SQL query causes an error
*/
@Test(description = "See OGC 12-128r14: Requirement 13 srs_id")
public void featureSRSconsistency() throws SQLException {
// This requirement is not currently called out under the TEST req area,
// but it is defined within Requirement 13 and not currently tested under
// the requirement 13 test code.
//
// Spatial Reference System ID:
// gpkg_spatial_ref_sys.srs_id; when data_type is features, SHALL also match
// gpkg_geometry_columns.srs_id;
int gpkgGeomCount = 0;
int gpkgContentCount = 0;
int gpkgAndContentCount = 0;
try (
final Statement statement2 = this.databaseConnection.createStatement();
final ResultSet resultSet2 = statement2.executeQuery(
String.format("SELECT count(*) as ggcCount FROM \'gpkg_geometry_columns\';"));
) {
gpkgGeomCount = resultSet2.getInt("ggcCount");
}
try (
final Statement statement2 = this.databaseConnection.createStatement();
final ResultSet resultSet2 = statement2.executeQuery(
String.format("SELECT count(*) as ggcCount FROM \'gpkg_contents\' WHERE data_type = \'features\';"));
) {
gpkgContentCount = resultSet2.getInt("ggcCount");
}
try (
final Statement statement2 = this.databaseConnection.createStatement();
final ResultSet resultSet2 = statement2.executeQuery(
String.format(
"SELECT count(*) as ggcCount FROM \'gpkg_contents\' INNER JOIN \'gpkg_geometry_columns\' ON ((gpkg_contents.srs_id = gpkg_geometry_columns.srs_id) AND (gpkg_contents.table_name = gpkg_geometry_columns.table_name));"));
) {
gpkgAndContentCount = resultSet2.getInt("ggcCount");
}
// If the geopackage geometry columns count is not the same as the geopackage content count (for features), then we have a problem.
if (gpkgGeomCount != gpkgContentCount) {
Assert.assertTrue(false,
ErrorMessage.format(ErrorMessageKeys.FEATURE_GEOMETRY_COLUMNS_DOES_NOT_MATCH_CONTENTS_COUNT));
// If we got a different count when looking at common feature names and srs_id values, then the test for requirement 13 fails
} else if (gpkgGeomCount != gpkgAndContentCount || gpkgContentCount != gpkgAndContentCount) {
Assert.assertTrue(false,
ErrorMessage.format(ErrorMessageKeys.FEATURE_GEOMETRY_COLUMNS_SRS_ID_NOT_CONSISTENT_WITH_CONTENTS));
}
}

Test fails because the results of two sql statements are not equal (first if):

  • SELECT count(*) as ggcCount FROM 'gpkg_geometry_columns';
    • returns 68
  • SELECT count(*) as ggcCount FROM 'gpkg_contents' WHERE data_type = 'features';
    • returns 34

The test expects that both counts are equal.

In http://www.geopackage.org/spec/ I cannot find a test case with id /base/core/contents/data/table_def/srs_id, and I don't see that Requirement 13 makes the test necessary.
Furthermore the if-else-statement in the end of the test can be simplified.

@jyutzler
Copy link
Contributor

jyutzler commented Jul 3, 2018

What is the test file that reproduces the false-positive?
BTW, this test was added as part of acbf3c5#diff-cd87833e776c4a923a7e470580031367

@ajanett
Copy link
Contributor

ajanett commented Jul 11, 2018

Since this test is not in the current spec, it is only implied by requirement 13; and due to the fact that the test is being added to the upcoming specification, I recommend that this version of the test be removed at this time until it can be added for the upcoming specification.

@lgoltz
Copy link
Contributor

lgoltz commented Aug 20, 2018

@ajanett, @jyutzler
Shall we remove the test completely or implement Requirement 13 as it is specified in 1.2?

@jyutzler
Copy link
Contributor

@lgoltz I do not feel comfortable answering this question until I can see a proper test case. Without it, I can't envision how we can go wrong.

@lgoltz
Copy link
Contributor

lgoltz commented Aug 23, 2018

@ajanett
Copy link
Contributor

ajanett commented Sep 26, 2018

The issue is that since srs_id is held in both gpkg_contents and gpkg_geometry_columns, it is possible for data integrity issues to occur.

The test is to ensure that for each table_name within gpkg_contents, verify that srs_id is consistent between gpkg_contents and gpkg_geometry_columns.

A better SQL for this test for the FEATURES is:
SELECT gpkg_geometry_columns.table_name, gpkg_geometry_columns.srs_id FROM gpkg_geometry_columns INNER JOIN gpkg_contents ON (gpkg_geometry_columns.table_name = gpkg_contents.table_name) WHERE (gpkg_contents.data_type = 'features' and gpkg_contents.srs_id != gpkg_geometry_columns.srs_id);
Fail if the SQL returns any rows

@ajanett
Copy link
Contributor

ajanett commented Sep 28, 2018

Requirement 146 was added to the specification to specifically address this issue and I believe that requirement is also coded into the ets-gpkg20. Therefore the FeatureSRSConsistency method may be removed and this issue can be closed.

@dstenger dstenger added this to In progress in CITE Apr 11, 2019
@lgoltz lgoltz moved this from In progress to To verify in CITE Apr 24, 2019
@lgoltz lgoltz moved this from To verify to Waiting in CITE Apr 24, 2019
@lgoltz lgoltz assigned ajanett and unassigned lgoltz and jyutzler Jul 31, 2019
@dstenger dstenger moved this from Waiting to To verify in CITE Mar 18, 2020
@dstenger dstenger assigned dstenger, lgoltz and jyutzler and unassigned ajanett Mar 18, 2020
@jyutzler
Copy link
Contributor

jyutzler commented Aug 8, 2020

My assessment is that the check comparing the row counts of gpkg_data_columns to row counts of gpkg_contents where datatype="features" is invalid because there may be extensions that use gpkg_data_columns with a different datatype.

This test file is a bit big to add to the ETS so I will reproduce the issue elsewhere.

Since there is already a test for R146 at FeaturesTests.featureGeometryColumnsDataValuesSrsId(), I see no need for the existing FeaturesTests.featureSRSconsistency().

jyutzler added a commit to jyutzler/ets-gpkg12 that referenced this issue Aug 8, 2020
@dstenger dstenger assigned keshavnangare and unassigned lgoltz and jyutzler Aug 10, 2020
CITE automation moved this from To verify to Done Aug 19, 2020
@keshavnangare keshavnangare reopened this Aug 19, 2020
CITE automation moved this from Done to In progress Aug 19, 2020
@keshavnangare
Copy link

@dstenger

We are fine with the solution and changes are successfully tested on the local environment.

@keshavnangare keshavnangare moved this from In progress to To verify in CITE Aug 19, 2020
CITE automation moved this from To verify to Done Aug 19, 2020
@dstenger dstenger added this to the 1.1 milestone Aug 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
CITE
  
Done
5 participants