[GRASS-dev] [GRASS GIS] #3235: Vect_cidx_find_all should not allow mixing GV_AREA with other geometries

#3235: Vect_cidx_find_all should not allow mixing GV_AREA with other geometries
-------------------------+-------------------------
Reporter: marisn | Owner: grass-dev@…
     Type: defect | Status: new
Priority: major | Milestone: 7.2.1
Component: LibVector | Version: svn-trunk
Keywords: | CPU: Unspecified
Platform: Unspecified |
-------------------------+-------------------------
At the moment Vect_cidx_find_all accepts any type mask. This leads to
unusable results in case if typemask allows both GV_LINES and GV_AREA in
the output. Reason - Vect_cidx_find_next will return line ID for GV_LINES
and area ID for GV_AREA. As both ID spaces are separate (thus might
contain identical ID values), mixing them in a single output structure is
a road to disaster.

The fix - Vect_cidx_find_all should fail to accept type mask matching
GV_LINES and GV_AREA (other combinations?) simultaneously.
Other option - change Vect_cidx_find_all to modify two structs - one with
line IDs and other with area IDs.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3235&gt;
GRASS GIS <https://grass.osgeo.org>

#3235: Vect_cidx_find_all should not allow mixing GV_AREA with other geometries
--------------------------+-------------------------
  Reporter: marisn | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: major | Milestone: 7.2.1
Component: LibVector | Version: svn-trunk
Resolution: | Keywords:
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------

Comment (by mmetz):

Replying to [ticket:3235 marisn]:
> At the moment Vect_cidx_find_all accepts any type mask. This leads to
unusable results in case if typemask allows both GV_LINES and GV_AREA in
the output. Reason - Vect_cidx_find_next will return line ID for GV_LINES
and area ID for GV_AREA. As both ID spaces are separate (thus might
contain identical ID values), mixing them in a single output structure is
a road to disaster.

That really is a disaster. The original implementation of the category
index did not take into account that area categories are added to the
category index. Probably because this is duplicate information, an area
can only have category values if it has a centroid with category values.
>
> The fix - Vect_cidx_find_all should fail to accept type mask matching
GV_LINES and GV_AREA (other combinations?) simultaneously.
> Other option - change Vect_cidx_find_all to modify two structs - one
with line IDs and other with area IDs.

The real fix would be to not add area category values to the category
index at all and update modules accordingly. This would be a substantial
change in the vector libraries, but it can be implemented without breaking
compatibility with G7. That would be a bit of work but I think it is worth
the effort because it avoids ID collision.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3235#comment:1&gt;
GRASS GIS <https://grass.osgeo.org>

#3235: Vect_cidx_find_all should not allow mixing GV_AREA with other geometries
--------------------------+-------------------------
  Reporter: marisn | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: major | Milestone: 7.2.4
Component: LibVector | Version: svn-trunk
Resolution: | Keywords:
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------

Comment (by martinl):

Still relevant?

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3235#comment:6&gt;
GRASS GIS <https://grass.osgeo.org>

#3235: Vect_cidx_find_all should not allow mixing GV_AREA with other geometries
--------------------------+-------------------------
  Reporter: marisn | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: major | Milestone: 8.0.0
Component: LibVector | Version: svn-trunk
Resolution: | Keywords:
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------
Changes (by marisn):

* milestone: 7.2.4 => 8.0.0

Comment:

Replying to [comment:6 martinl]:
> Still relevant?

Complex code doesn't fix itself by accident. Yes, still relevant.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3235#comment:7&gt;
GRASS GIS <https://grass.osgeo.org>

#3235: Vect_cidx_find_all should not allow mixing GV_AREA with other geometries
--------------------------+-------------------------
  Reporter: marisn | Owner: grass-dev@…
      Type: defect | Status: closed
  Priority: major | Milestone: 8.0.0
Component: LibVector | Version: svn-trunk
Resolution: fixed | Keywords:
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------
Changes (by mmetz):

* status: new => closed
* resolution: => fixed

Comment:

In [changeset:"74446" 74446]:
{{{
#!CommitTicketReference repository="" revision="74446"
vectorlib: Vect_cidx_find_all() should not allow mixing GV_AREA with other
geometries, fixes #3235
}}}

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3235#comment:8&gt;
GRASS GIS <https://grass.osgeo.org>

#3235: Vect_cidx_find_all should not allow mixing GV_AREA with other geometries
--------------------------+-------------------------
  Reporter: marisn | Owner: grass-dev@…
      Type: defect | Status: closed
  Priority: major | Milestone: 8.0.0
Component: LibVector | Version: svn-trunk
Resolution: fixed | Keywords:
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------

Comment (by mmetz):

In [changeset:"74447" 74447]:
{{{
#!CommitTicketReference repository="" revision="74447"
vectorlib: Vect_cidx_find_all() should not allow mixing GV_AREA with other
geometries, fixes #3235 (backport trunk r74446)
}}}

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3235#comment:9&gt;
GRASS GIS <https://grass.osgeo.org>

#3235: Vect_cidx_find_all should not allow mixing GV_AREA with other geometries
--------------------------+-------------------------
  Reporter: marisn | Owner: grass-dev@…
      Type: defect | Status: closed
  Priority: major | Milestone: 7.6.2
Component: LibVector | Version: svn-trunk
Resolution: fixed | Keywords:
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------
Changes (by neteler):

* milestone: 8.0.0 => 7.6.2

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3235#comment:10&gt;
GRASS GIS <https://grass.osgeo.org>

#3235: Vect_cidx_find_all should not allow mixing GV_AREA with other geometries
--------------------------+-------------------------
  Reporter: marisn | Owner: grass-dev@…
      Type: defect | Status: closed
  Priority: major | Milestone: 8.0.0
Component: LibVector | Version: svn-trunk
Resolution: fixed | Keywords:
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------
Changes (by mmetz):

* milestone: 7.6.2 => 8.0.0

Comment:

Modules using `Vect_cidx_find_all()` are v.profile, v.out.ogr,
v.db.select, v.net, v.edit, and none of them mixes GV_AREA with other
geometries. A fatal error will occur if a new module does mix GV_AREA with
other geometries when calling `Vect_cidx_find_all()` (programmer's error).

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3235#comment:11&gt;
GRASS GIS <https://grass.osgeo.org>

#3235: Vect_cidx_find_all should not allow mixing GV_AREA with other geometries
--------------------------+-------------------------
  Reporter: marisn | Owner: grass-dev@…
      Type: defect | Status: closed
  Priority: major | Milestone: 8.0.0
Component: LibVector | Version: svn-trunk
Resolution: fixed | Keywords:
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------

Comment (by neteler):

Replying to [comment:11 mmetz]:
> Modules using `Vect_cidx_find_all()` are v.profile, v.out.ogr,
v.db.select, v.net, v.edit, and none of them mixes GV_AREA with other
geometries. A fatal error will occur if a new module does mix GV_AREA with
other geometries when calling `Vect_cidx_find_all()` (programmer's error).

Then the bug report needs to be reopened again since we didn't reach 8.0.0
yet.
Say, closing it with changes applied to 7.6.2 while the milestone is 8 is
confusing (for me)...

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3235#comment:12&gt;
GRASS GIS <https://grass.osgeo.org>

#3235: Vect_cidx_find_all should not allow mixing GV_AREA with other geometries
--------------------------+-------------------------
  Reporter: marisn | Owner: grass-dev@…
      Type: defect | Status: closed
  Priority: major | Milestone: 7.6.2
Component: LibVector | Version: svn-trunk
Resolution: fixed | Keywords:
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------
Changes (by mmetz):

* milestone: 8.0.0 => 7.6.2

Comment:

Replying to [comment:12 neteler]:
> Replying to [comment:11 mmetz]:
> > Modules using `Vect_cidx_find_all()` are v.profile, v.out.ogr,
v.db.select, v.net, v.edit, and none of them mixes GV_AREA with other
geometries. A fatal error will occur if a new module does mix GV_AREA with
other geometries when calling `Vect_cidx_find_all()` (programmer's error).
>
> Then the bug report needs to be reopened again since we didn't reach
8.0.0 yet.
> Say, closing it with changes applied to 7.6.2 while the milestone is 8
is confusing (for me)...

Sorry, that was trac confusion because we were editing the same time.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3235#comment:13&gt;
GRASS GIS <https://grass.osgeo.org>