Bad nesting of optional query chains

[der]

Example query generated by Elda:

PREFIX geo: <http://www.w3.org/2003/01/geo/wgs84_pos#>
PREFIX dct: <http://purl.org/dc/terms/>
PREFIX def-som: <http://environment.data.gov.uk/def/bwq-som/>
PREFIX rdf: <http://www.w3.org/1999/02/22-rdf-syntax-ns#>
PREFIX xsd: <http://www.w3.org/2001/XMLSchema#>
PREFIX skos: <http://www.w3.org/2004/02/skos/core#>
PREFIX spatialrelations: <http://data.ordnancesurvey.co.uk/ontology/spatialrelations/>
PREFIX def-ef: <http://location.data.gov.uk/def/ef/SamplingPoint/>
PREFIX def-stp: <http://environment.data.gov.uk/def/bwq-stp/>
PREFIX def-bw: <http://environment.data.gov.uk/def/bathing-water/>

SELECT DISTINCT ?item WHERE
{
  {
    BIND(xsd:dateTime(replace(str(now()), "\\\\..*$", "")) AS ?doi)
    {
      ?som def-bw:bathingWater ?bwt .
      ?som def-som:startOfSuspension ?start
      FILTER NOT EXISTS {?som dct:isReplacedBy ?som2 }
    }
    OPTIONAL { ?som def-som:endOfSuspension ?end }
    BIND(?som AS ?item)
    FILTER ( ( ( bound(?end) && ( ?start <= ?doi ) ) && ( ?doi <= ?end ) ) || ( ( ! bound(?end) ) && ( ?start <= ?doi ) ) )
  }
  UNION
  {
    BIND(xsd:dateTime(replace(str(now()), "\\\\..*$", "")) AS ?doi)
    ?stp def-stp:riskLevel def-stp:increased .
    ?stp def-stp:bathingWater ?bwt .
    ?stp def-stp:publishedAt ?padt .
    ?stp def-stp:expiresAt ?end .
    ?stp def-stp:predictedAt ?start
    FILTER ( ( ?start <= ?doi ) && ( ?doi <= ?end ) )
    FILTER NOT EXISTS {
      ?stp2 def-stp:bathingWater ?bwt .
      ?stp2 def-stp:publishedAt ?padt2 .
      ?stp2 def-stp:expiresAt ?end2 .
      ?stp2 def-stp:predictedAt ?start2
      FILTER ( ( ( ?padt2 > ?padt ) && ( ?start2 <= ?doi ) ) && ( ?doi <= ?end2 ) )
    }
    BIND(?stp AS ?item)
  }
  UNION
  {
    BIND(xsd:dateTime(replace(str(now()), "\\\\..*$", "")) AS ?doi)
    {
      ?pi rdf:type def-som:PollutionIncident .
      ?pi def-bw:bathingWater ?bwt .
      ?pi def-som:startOfIncident ?start
      FILTER NOT EXISTS { ?pi dct:isReplacedBy ?other }
    }
    OPTIONAL { ?pi def-som:endOfIncident ?end }
    FILTER ( ( ?start <= ?doi ) && ( ( ! bound(?end) ) || ( ?doi <= ?end ) ) )
    BIND(?pi AS ?item)
  }
  OPTIONAL { ?item def-bw:bathingWater ?___1 . ?___1 rdf:type ?___0 }
  OPTIONAL { ?item def-bw:regionalOrganization ?___3 . ?___3 rdf:type ?___2 }
  OPTIONAL { ?item def-ef:samplingPoint ?___5 . ?___5 spatialrelations:northing ?___4 }
  OPTIONAL { ?___0 skos:prefLabel ?___6 }
  OPTIONAL { ?___4 geo:long ?___7 }
  OPTIONAL { ?item def-som:expectedEndOfSuspension ?___8 }
}
ORDER BY ?___0 ?___2 DESC(?___4) ?___6 DESC(?___7) DESC(?___8) ?item
OFFSET 100 LIMIT 50

Example rewritten form which avoids unbounds in nested optionals:

PREFIX geo: <http://www.w3.org/2003/01/geo/wgs84_pos#>
PREFIX dct: <http://purl.org/dc/terms/>
PREFIX def-som: <http://environment.data.gov.uk/def/bwq-som/>
PREFIX rdf: <http://www.w3.org/1999/02/22-rdf-syntax-ns#>
PREFIX xsd: <http://www.w3.org/2001/XMLSchema#>
PREFIX skos: <http://www.w3.org/2004/02/skos/core#>
PREFIX spatialrelations: <http://data.ordnancesurvey.co.uk/ontology/spatialrelations/>
PREFIX def-ef: <http://location.data.gov.uk/def/ef/SamplingPoint/>
PREFIX def-stp: <http://environment.data.gov.uk/def/bwq-stp/>
PREFIX def-bw: <http://environment.data.gov.uk/def/bathing-water/>

SELECT DISTINCT ?item WHERE {
  {
    BIND(xsd:dateTime(REPLACE(STR(NOW()), "\\\\..*$", "")) AS ?doi)
    {
      ?som def-bw:bathingWater ?bwt ;
        def-som:startOfSuspension ?start .
      FILTER(NOT EXISTS {?som dct:isReplacedBy ?som2 . })
    }
    OPTIONAL { ?som def-som:endOfSuspension ?end . }
    BIND(?som AS ?item)
    FILTER ( ( ( (BOUND(?end)) && ( ?start <= ?doi ) ) && ( ?doi <= ?end ) ) || ( ( ! (BOUND(?end)) ) && ( ?start <= ?doi ) ) )
  }
  UNION
  {
    BIND(xsd:dateTime(REPLACE(STR(NOW()), "\\\\..*$", "")) AS ?doi)
    ?stp def-stp:riskLevel def-stp:increased ;
      def-stp:bathingWater ?bwt ;
      def-stp:publishedAt ?padt ;
      def-stp:expiresAt ?end ;
      def-stp:predictedAt ?start .
    FILTER ( ( ?start <= ?doi ) && ( ?doi <= ?end ) )
    FILTER(NOT EXISTS {
      ?stp2 def-stp:bathingWater ?bwt ;
        def-stp:publishedAt ?padt2 ;
        def-stp:expiresAt ?end2 ;
        def-stp:predictedAt ?start2 .
      FILTER ( ( ( ?padt2 > ?padt ) && ( ?start2 <= ?doi ) ) && ( ?doi <= ?end2 ) )
    })
    BIND(?stp AS ?item)
  }
  UNION
  {
    BIND(xsd:dateTime(REPLACE(STR(NOW()), "\\\\..*$", "")) AS ?doi)
    {
      ?pi rdf:type def-som:PollutionIncident ;
        def-bw:bathingWater ?bwt ;
        def-som:startOfIncident ?start .
      FILTER(NOT EXISTS { ?pi dct:isReplacedBy ?other. })
    }
    OPTIONAL { ?pi def-som:endOfIncident ?end. }
    FILTER ( ( ?start <= ?doi ) && ( ( ! (BOUND(?end)) ) || ( ?doi <= ?end ) ) )
    BIND(?pi AS ?item)
  }
  OPTIONAL {
    ?item def-bw:bathingWater ?___1 .
    ?___1 rdf:type ?___0.
    OPTIONAL { ?___0 skos:prefLabel ?___6. }
  }
  OPTIONAL {
    ?item def-bw:regionalOrganization ?___3 .
    ?___3 rdf:type ?___2.
  }
  OPTIONAL {
    ?item def-ef:samplingPoint ?___5 .
    ?___5 spatialrelations:northing ?___4.
    OPTIONAL { ?___4 geo:long ?___7. }
  }
  OPTIONAL { ?item def-som:expectedEndOfSuspension ?___8. }
}
ORDER BY (?___0) (?___2) DESC(?___4) (?___6) DESC(?___7) DESC(?___8) (?item)
OFFSET 100
LIMIT 50

[simonoakesepimorphics]

This query pattern is created by APIQuery.generateSortVariable which calls APIQuery.optionalProperty. The fix will require us to revise the query writing process for sorting and the construct which represents the query fragments (RDFQ.Triple). The new construct must support representations of nested query fragments.

A similar query pattern exists in APIQuery.addPropertyHasntValue (ie. filtering by a property being unbound) so we should probably correct this at the same time.

[kal]

@simonoakesepimorphics - do you have a rough estimate of the effort required to address this?

[simonoakesepimorphics]

I estimate 1-2 days.

[der]

Just realised that the PR for this merged into master.

I think we'll need a hotfix version based on the release-2.0.3 branch so as not to depend on the jena/java uplifts.

[kal]

@der, @simonoakesepimorphics - I think this was fixed in the 2.0.4 release. Is that correct?

[simonoakesepimorphics]

Yes, I ported this fix to release/2.0.4 and released the version.

[der]

Can we have just one branch for 2.x support, presumably maintenance/2.x? We tag releases so can easily go back to a specific point release if we need to without having lots of versioned branches.

[simonoakesepimorphics]

Sure, I will create that.

[der]

Sure, I will create that.

Think @kal has already created it but not sure if it includes 2.0.4

[kal]

Yes it does