Torque
  1. Torque
  2. TORQUE-39

Wrong database name in generated XML schema with jdbc task

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.2
    • Fix Version/s: 4.0-beta1
    • Component/s: Generator
    • Labels:
      None

      Description

      In org.apache.torque.task.TorqueJDBCTransformTask#generateXML() is a bug in line 196.

      195: databaseNode = doc.createElement("database");
      196: databaseNode.setAttribute("name", dbUser);

      This would generate a wrong XML schema <database name="dbUser"> instead of <database name="databaseName">...

        Issue Links

          Activity

          Hide
          Thomas Fox added a comment -

          Where would you expect the "databaseName" to come from ? To my knowledge, nowhere in the generator exists the concept of a database name, so one has to be invented. In my opinion, a sensible default is the name of the database user

          Show
          Thomas Fox added a comment - Where would you expect the "databaseName" to come from ? To my knowledge, nowhere in the generator exists the concept of a database name, so one has to be invented. In my opinion, a sensible default is the name of the database user
          Hide
          Thoralf Rickert added a comment -

          Is it a big problem to hand over a property like "torque.databaseName" to the task or to parse the property "torque.database.url"?

          Show
          Thoralf Rickert added a comment - Is it a big problem to hand over a property like "torque.databaseName" to the task or to parse the property "torque.database.url"?
          Hide
          CG Monroe added a comment -

          A couple of things to keep in mind about this.

          First, the database tag name attribute is a Torque id and not actually tied to any physical DB. For example, I have multiple Torque schema files for tables that all exist in the same physical DB but are used by subsections of my code.

          Second, IMHO the schema files generated via JDBC should be considered a starting point for creating production ready files. There is just too many options that JDBC can know nothing about such as idmethod, java naming methods, inheritance, and the like. So the generated files will always need some hand tweaking to be right.

          FWIW, I don't think there is a "right" answer to what goes in the generated name attribute. I could make the argument that as an MSSQL user who can only create schema files by specifying the owner of the tables that the dbuser is the correct value to use. Oracle users might claim they want the Oracle schema being mapped used. Who's right?

          Maybe the best solution is to just used a static value like "jdbc-generated"? Perhaps this should be a new property in the default.properties that can be overridden in the build.properties?

          Show
          CG Monroe added a comment - A couple of things to keep in mind about this. First, the database tag name attribute is a Torque id and not actually tied to any physical DB. For example, I have multiple Torque schema files for tables that all exist in the same physical DB but are used by subsections of my code. Second, IMHO the schema files generated via JDBC should be considered a starting point for creating production ready files. There is just too many options that JDBC can know nothing about such as idmethod, java naming methods, inheritance, and the like. So the generated files will always need some hand tweaking to be right. FWIW, I don't think there is a "right" answer to what goes in the generated name attribute. I could make the argument that as an MSSQL user who can only create schema files by specifying the owner of the tables that the dbuser is the correct value to use. Oracle users might claim they want the Oracle schema being mapped used. Who's right? Maybe the best solution is to just used a static value like "jdbc-generated"? Perhaps this should be a new property in the default.properties that can be overridden in the build.properties?
          Hide
          Thomas Fox added a comment -

          I am not convinced about adding a new property.
          a) it is a lot of work. The ant, maven1 and maven2 part need to be considered.
          b) We have already a lot of properties. Is this worth yet another one ?

          My suggestion is the following:

          • If someone finds the new property is worth the trouble and creates a patch, I'd test and commit it.
          • If not, I'd change the database name from the db username to "default". It will work in the most setups, and if someone does something unusual, he has to change the database name anyway.

          If you want to create a patch, please indicate so in the next few days.

          Show
          Thomas Fox added a comment - I am not convinced about adding a new property. a) it is a lot of work. The ant, maven1 and maven2 part need to be considered. b) We have already a lot of properties. Is this worth yet another one ? My suggestion is the following: If someone finds the new property is worth the trouble and creates a patch, I'd test and commit it. If not, I'd change the database name from the db username to "default". It will work in the most setups, and if someone does something unusual, he has to change the database name anyway. If you want to create a patch, please indicate so in the next few days.
          Hide
          ivano luberti added a comment - - edited

          Thomas, you just asked me to create here an issue about this matter but it seems somethin that has been already pointed out.
          But it is so old you maybe have forgotten about it

          I would only add a consideration to the ones above: when classes are generated using the wrong db name, that name is used by Torque runtime when it tries to connect to the db.
          I think that coherence should be guaranteed between the way generator and runtime connect to the same database

          Show
          ivano luberti added a comment - - edited Thomas, you just asked me to create here an issue about this matter but it seems somethin that has been already pointed out. But it is so old you maybe have forgotten about it I would only add a consideration to the ones above: when classes are generated using the wrong db name, that name is used by Torque runtime when it tries to connect to the db. I think that coherence should be guaranteed between the way generator and runtime connect to the same database
          Hide
          Thomas Fox added a comment -

          The default schema name is now "default".
          It can be overridden by setting the torque.database.name option

          Show
          Thomas Fox added a comment - The default schema name is now "default". It can be overridden by setting the torque.database.name option

            People

            • Assignee:
              Thomas Fox
              Reporter:
              Thoralf Rickert
            • Votes:
              1 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development