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.

Sign up for GitHub

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

Jump to bottom

PostgreSQL sink creates table when entity id contains special characters #2220

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

PostgreSQL sink creates table when entity id contains special characters #2220

wants to merge 1 commit into from

Conversation

kajal583
Copy link
Contributor

This PR fixes issue#1977 i.e case1 and case2
CASE 1: Encoding is enabled
Postgresql sink creates table for all datamodel. But found bug in table name of some data model as shown below:

Data model Table name Expected Table name
dm-by-attribute x002fkajalxffffurnx003angsix002dldxffffx0052oomxfffftemperature x002fkajalxffffurnx003angsix002dldxffffroomxfffftemperature
dm-by-entity x002fkajalxffffurnx003angsix002dldxffffx0052oom x002fkajalxffffurnx003angsix002dldxffffroom
dm-by-entity-database x002fkajalxffffurnx003angsix002dldxffffx0052oom x002fkajalxffffurnx003angsix002dldxffffroom
dm-by-fixed-entity-type x0052oom room
dm-by-fixed-entity-type-database x0052oom room
dm-by-entity-database-schema x002fkajalxffffurnx003angsix002dldxffffx0052oom x002fkajalxffffurnx003angsix002dldxffffroom
dm-by-fixed-entity-type-database-schema x0052oom room

CASE 2: Encoding is disabled
In this case, postgresql sink create table only for below data models:

  • dm-by-fixed-entity-type-database-schema
  • dm-by-entity-type-database-schema
  • dm-by-fixed-entity-type-database
  • dm-by-fixed-entity-type
  • dm-by-entity-type-database
  • dm-by-entity-type
  • dm-by-service-path

and for other data models (dm-by-entity,dm-by-attribute,dm-by-entity-database,dm-by-entity-database-schema) it does not create tables.

@fgalan
Copy link
Member

fgalan commented Feb 21, 2023

Thank you for your contribution!

We hope we can provide feedback on it in a short time

@mapedraza
Copy link
Collaborator

@Kajal-NEC Please, could you provide the entity name, entity type, fiware-service and fiware-servicepath header you are using to create the DB for each datamodel?

@kajal583
Copy link
Contributor Author

Please find below details:

  • entity name=urn:ngsi-ld
  • entity type: Room
  • fiware-service= kajal
  • fiware-servicepath=/kajal

fgalan
fgalan reviewed Mar 1, 2023
View reviewed changes
cygnus-ngsi/src/main/java/com/telefonica/iot/cygnus/sinks/NGSIPostgreSQLSink.java
@@ -616,32 +616,32 @@ public String buildTableName(String servicePath, String entity, String entityTyp
+ "dm-by-service-path data model");
} // if

name = NGSIUtils.encode(servicePath, true, false);
name = NGSIUtils.encodePostgresTable(servicePath);
Copy link
Member

Choose a reason for hiding this comment

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

After the changes in this PR, is encode() still used in other parts of the code? Or it can be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

encode() is used in below part of the code:
When "enable encoding=false" that time encode() is used to build DBName and SchemaName in class NGSIPostgreSQLSink. Also it is used in other sink like NGSIPostgisSink, NGSIOracleSink,NGSIMysqlSink etc. So we can't remove it.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the clarification

NTC

fgalan
fgalan reviewed Mar 1, 2023
View reviewed changes
cygnus-ngsi/src/main/java/com/telefonica/iot/cygnus/utils/NGSICharsets.java
@@ -41,9 +41,9 @@ public static String encodePostgreSQL(String in) {
char c = in.charAt(i);
Copy link
Member

Choose a reason for hiding this comment

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

Where encodePostgreSQL() function is used?

It seems this PR fixes it but it is not clear to me where it is used...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When "Enable ecoding=true", encodePostgreSQL() function is used while building schema name, db name and table name.

@mapedraza
Copy link
Collaborator

Organising a bit the information:

Parameters used

entity name=urn:ngsi-ld
entity type: Room
fiware-service= kajal
fiware-servicepath=/kajal

Datamodels analysis:

dm-by-attribute
Datamodel name: servicepath_entityname_entitytype_attribute

x002fkajalxffffurnx003angsix002dldxffffx0052oomxfffftemperature
x002fkajalxffffurnx003angsix002dldxffffroomxfffftemperature

Differences: R capital (x0052 instead of r)

dm-by-entity
Datamodel name: servicepath_entityname_entitytype

x002fkajalxffffurnx003angsix002dldxffffx0052oom
x002fkajalxffffurnx003angsix002dldxffffroom

Differences: R capital (x0052 instead of r)

dm-by-entity-database
Datamodel name: servicepath_entityname_entitytype

x002fkajalxffffurnx003angsix002dldxffffx0052oom
x002fkajalxffffurnx003angsix002dldxffffroom

Differences: R capital (x0052 instead of r)

dm-by-fixed-entity-type
Datamodel name: entitytype

x0052oom
room

Differences: R capital (x0052 instead of r)

dm-by-fixed-entity-type-database
Datamodel name: entitytype

x0052oom
room

Differences: R capital (x0052 instead of r)

dm-by-entity-database-schema
Datamodel name: servicepath_entityname_entitytype

x002fkajalxffffurnx003angsix002dldxffffx0052oom
x002fkajalxffffurnx003angsix002dldxffffroom

Differences: R capital (x0052 instead of r)

dm-by-fixed-entity-type-database-schema
Datamodel name: entitytype

x0052oom
room

Differences: R capital (x0052 instead of r)

Conclusion

As far as I can see, this PR only fix capitals problems. @Kajal-NEC How this PR approach the issue #1977 regarding the - sign?

@kajal583
Copy link
Contributor Author

kajal583 commented Mar 22, 2023 •
edited
Loading

Organising a bit the information:

Parameters used

entity name=urn:ngsi-ld
entity type: Room
fiware-service= kajal
fiware-servicepath=/kajal

Differences: R capital (x0052 instead of r)

Conclusion

As far as I can see, this PR only fix capitals problems. @Kajal-NEC How this PR approach the issue #1977 regarding the - sign?

@mapedraza , Case 2 mentioned above in PR description is regarding issue #1977.
When encoding is disabled and entityID containes special character (i.e '-'), postgresql sink does not create table for some data model shown in below table.
Parameter used:
entity name=urn:ngsi-ld entity type: Room fiware-service= kajal fiware-servicepath=/kajal

DATA MODEL Table_name before applying this patch Table name after applying this patch
dm-by-fixed-entity-type-database-schema room room
dm-by-entity-type-database-schema kajal_room kajal_room
dm-by-fixed-entity-type-database room room
dm-by-fixed-entity-type room room
dm-by-entity-type-database kajal_room kajal_room
dm-by-entity-type kajal_room kajal_room
dm-by-service-path kajal kajal
dm-by-entity-database-schema Table not created kajal_urn_ngsi_ld_room
dm-by-entity-database Table not created kajal_urn_ngsi_ld_room
dm-by-entity Table not created kajal_urn_ngsi_ld_room
dm-by-attribute Table not created kajal_urn_ngsi_ld_room_temperature

@fgalan
Copy link
Member

fgalan commented Apr 27, 2023

We wonder how this PR would affect to existing deployments (i.e. backward compatibility). Current databases using table names like x0052oom would be potentially impacted if this PR is applied.

In order to know the scope of the change, are your modifications changing always the table names? Or only when enable_encoding is enabled? Could you add a enable_encoding column to your tables (considering both cases false and true)?

Thanks!

@kajal583
Copy link
Contributor Author

We wonder how this PR would affect to existing deployments (i.e. backward compatibility). Current databases using table names like x0052oom would be potentially impacted if this PR is applied.

In order to know the scope of the change, are your modifications changing always the table names? Or only when enable_encoding is enabled? Could you add a enable_encoding column to your tables (considering both cases false and true)?

Thanks!

CASE 1 from PR discription:
When enable_encoding=true, postgresql sink creates table. But when there is CAPITAL letter in tableName then it gets replaced by some code like x0041, x0042, x0043, x0044 etc as shown in below table:
enable_encoding=true
Parameter used:
entity name= urn:ngsi-ld entity type= Room fiware-service= kajal fiware-servicepath= /kajal

Datamodel name Expected table name as per Documentation Table Name before applying PR Table Name after applying PR Differences
dm-by-attribute x002f{svcPath}xffff{entityId}xffff{entityType}xffff{attribute} x002fkajalxffffurnx003angsix002dldxffffx0052oomxfffftemperature x002fkajalxffffurnx003angsix002dldxffffroomxfffftemperature R capital (x0052 instead of r)
dm-by-entity x002f{svcPath}xffff{entityId}xffff{entityType} x002fkajalxffffurnx003angsix002dldxffffx0052oom x002fkajalxffffurnx003angsix002dldxffffroom R capital (x0052 instead of r)
dm-by-entity-database x002f{svcPath}xffff{entityId}xffff{entityType} x002fkajalxffffurnx003angsix002dldxffffx0052oom x002fkajalxffffurnx003angsix002dldxffffroom R capital (x0052 instead of r)
dm-by-fixed-entity-type {entityType} x0052oom room **R capital (x0052 instead of r)
dm-by-fixed-entity-type-database {entityType} x0052oom room R capital (x0052 instead of r)
dm-by-entity-database-schema x002f{svcPath}xffff{entityId}xffff{entityType} x002fkajalxffffurnx003angsix002dldxffffx0052oom x002fkajalxffffurnx003angsix002dldxffffroom R capital (x0052 instead of r)
dm-by-fixed-entity-type-database-schema {entityType} x0052oom room R capital (x0052 instead of r)
dm-by-entity-type-database-schema x002f{svcPath}xffff{entityType} x002fkajalxffffroom x002fkajalxffffroom No change
dm-by-entity-type-database x002f{svcPath}xffff{entityType} x002fkajalxffffroom x002fkajalxffffroom No change
dm-by-entity-type x002f{svcPath}xffff{entityType} x002fkajalxffffroom x002fkajalxffffroom No change
dm-by-service-path x002f{svcPath} x002fkajal x002fkajal No change

Case 2 From PR discription:
Case 2 mentioned in PR description is fix for #1977.
Issue #1977 occurs when enable_encoding=false
Describe issue #1977: When enable_encoding=false and entityID containes special character (i.e '-'), postgresql sink does not create table for some data model shown in below table.
enable_encoding=false
Parameter used:
entity name= urn:ngsi-ld entity type= Room fiware-service= kajal fiware-servicepath= /kajal

DATA MODEL Expected table name as per Documentation Table_name before applying this patch Table name after applying this patch
dm-by-fixed-entity-type-database-schema {entityType} room room
dm-by-entity-type-database-schema {servicepath_entitytype} kajal_room kajal_room
dm-by-fixed-entity-type-database {entitytype} room room
dm-by-fixed-entity-type {entitytype} room room
dm-by-entity-type-database {servicepath_entitytype} kajal_room kajal_room
dm-by-entity-type {servicepath_entitytype} kajal_room kajal_room
dm-by-service-path {entitytype} kajal kajal
dm-by-entity-database-schema {servicepath_entityname_entitytype} Table not created kajal_urn_ngsi_ld_room
dm-by-entity-database {servicepath_entityname_entitytype} Table not created kajal_urn_ngsi_ld_room
dm-by-entity {servicepath_entityname_entitytype} Table not created kajal_urn_ngsi_ld_room
dm-by-attribute {servicepath_entityname_entitytype_attribute} Table not created kajal_urn_ngsi_ld_room_temperature

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fgalan fgalan fgalan left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

PostgreSQL sink does not create table when entity id contains special characters
3 participants
@kajal583 @fgalan @mapedraza