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

Add test case gml namespace, missing will cause a warning. #751

Closed
wants to merge 1 commit into from

Conversation

w345731923
Copy link
Contributor

@w345731923 w345731923 commented Nov 10, 2023

gml does not add the namespace when converting, which causes libxml2 to throw a warning.
postgis 3.4.0
CGAL 5.4.5
PROJ="8.2.1
LIBXML="2.9.11"
LIBJSON="0.15"

2023-11-10 12:18:36.706 CST [23404] LOG: execute : SET SESSION search_path TO 'public'
namespace warning : Namespace prefix gml is not defined
<gml:LineString srsName="EPSG:4269">
^
2023-11-10 12:18:36.715 CST [23404] LOG: execute : SELECT ST_GeomFromGML($$
<gml:LineString srsName="EPSG:4269">
gml:coordinates
-71.16028,42.258729 -71.160837,42.259112 -71.161143,42.25932
</gml:coordinates>
</gml:LineString>
$$)

reference https://www.ogc.org/standard/gml/
《 5.1 XML namespaces 》

All components of the GML schema are defined in the namespace with the identifier "http://www.opengis.net/gml/3.2", for which the prefix gml or the default namespace is used within this International Standard.

2023-11-10 12:29:49.619 CST [23404] LOG: execute : SELECT ST_GeomFromGML($$
<gml:LineString xmlns:gml="http://www.opengis.net/gml"
srsName="EPSG:4269">
gml:coordinates
-71.16028,42.258729 -71.160837,42.259112 -71.161143,42.25932
</gml:coordinates>
</gml:LineString>
$$)

@w345731923
Copy link
Contributor Author

@strk

Hi Sandro,
Something went wrong with regression testing.
https://github.com/postgis/postgis/actions/runs/6820765253/job/18550181118?pr=751

regress/core/regress_buffer_params .. failed (diff expected obtained: /tmp/pgis_reg/test_51_diff)

--- ./regress/core/regress_buffer_params_expected 2023-11-10 04:30:53.558070129 +0000
+++ /tmp/pgis_reg/test_51_out 2023-11-10 04:32:38.021623410 +0000
@@ -1,5 +1,5 @@
point quadsegs=2|POLYGON((1 0,0.7071 -0.7071,0 -1,-0.7071 -0.7071,-1 0,-0.7071 0.7071,0 1,0.7071 0.7071,1 0))
-line quadsegs=2|POLYGON((10 2,11.414 1.414,12 0,11.414 -1.414,10 -2,0 -2,-1.414 -1.414,-2 2.449e-16,-1.414 1.414,0 2,10 2))
+line quadsegs=2|POLYGON((10 2,11.414 1.414,12 0,11.414 -1.414,10 -2,0 -2,-1.414 -1.414,-2 0,-1.414 1.414,0 2,10 2))
line quadsegs=2 endcap=flat|POLYGON((10 2,10 -2,0 -2,0 2,10 2))
line quadsegs=2 endcap=butt|POLYGON((10 2,10 -2,0 -2,0 2,10 2))
line quadsegs=2 endcap=square|POLYGON((10 2,12 2,12 -2,0 -2,-2 -2,-2 2,10 2))

Will modifying the example affect regression testing?
I did not modify the regress_buffer_params_expected and regress_buffer_params.sql files.

@pramsey
Copy link
Member

pramsey commented Nov 10, 2023

rebase from master, should fix the CI failure.

@pramsey
Copy link
Member

pramsey commented Nov 10, 2023

I don't find that omitting the namespace causes an error.

SELECT ST_GeomFromGML($$
    <gml:LineString  
                        srsName="EPSG:4269">
        <gml:coordinates>
            -71.16028,42.258729 -71.160837,42.259112 -71.161143,42.25932
        </gml:coordinates>
    </gml:LineString>
$$);

@w345731923
Copy link
Contributor Author

I don't find that omitting the namespace causes an error.

SELECT ST_GeomFromGML($$
    <gml:LineString  
                        srsName="EPSG:4269">
        <gml:coordinates>
            -71.16028,42.258729 -71.160837,42.259112 -71.161143,42.25932
        </gml:coordinates>
    </gml:LineString>
$$);

When logging_collector=on, it will be printed in the log but not on the console. Please check the logs.
When logging_collector=off, warnings are printed in psql.

@w345731923
Copy link
Contributor Author

I found the warning location.
Reference https://gitlab.gnome.org/GNOME/libxml2/-/blob/master/SAX2.c
Line 1641

@strk
Copy link
Member

strk commented Nov 13, 2023

The namespace definition should be in the document header, we don't want to repeat it on every geometry.
The function supports tweaking the namespace being used too, in case you want it empty (do you get the warning if it's empty?)
https://postgis.net/docs/ST_AsGML.html

Copy link
Member

@strk strk left a comment

Choose a reason for hiding this comment

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

I'm actually ok with the change, doesn't hurt for sure

@w345731923
Copy link
Contributor Author

The namespace definition should be in the document header, we don't want to repeat it on every geometry. The function supports tweaking the namespace being used too, in case you want it empty (do you get the warning if it's empty?) https://postgis.net/docs/ST_AsGML.html

ST_AsGML is normal, but warnings are issued during conversion, such as ST_GeomFromGML.

@strk
Copy link
Member

strk commented Nov 14, 2023

I've merged your change as of dc000e9 - thank you

@strk strk closed this Nov 14, 2023
robe2 pushed a commit that referenced this pull request Nov 14, 2023
Missing namespace definition will cause a warning
See #751
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants