Avro
  1. Avro
  2. AVRO-98

Make C++ parsing comply to spec

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.1.0
    • Component/s: c++
    • Labels:
      None

      Description

      Currently the C++ parser has built in assumptions about the order of attributes, and does not allow for extra meta-data attributes.
      I have a fix for this in progress, so I'm opening the issue now (just so people know it's coming).

      1. AVRO-98-revised.patch
        48 kB
        Scott Banachowski
      2. applypatch98.sh
        0.2 kB
        Scott Banachowski

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Resolved Resolved
        15d 21h 58m 1 Doug Cutting 03/Sep/09 17:23
        Resolved Resolved Closed Closed
        13d 6h 8m 1 Doug Cutting 16/Sep/09 23:31
        Doug Cutting made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Doug Cutting made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Assignee Scott Banachowski [ sbanacho ]
        Fix Version/s 1.1.0 [ 12314106 ]
        Resolution Fixed [ 1 ]
        Hide
        Doug Cutting added a comment -

        I just committed this. Thanks, Scott!

        Show
        Doug Cutting added a comment - I just committed this. Thanks, Scott!
        Scott Banachowski made changes -
        Attachment AVRO-98.patch [ 12418291 ]
        Scott Banachowski made changes -
        Attachment AVRO-98-revised.patch [ 12418459 ]
        Hide
        Scott Banachowski added a comment -

        new patch to replace the original one, now works with boost 1.33 and higher

        Show
        Scott Banachowski added a comment - new patch to replace the original one, now works with boost 1.33 and higher
        Hide
        Doug Cutting added a comment -

        I have boost 1.34.1, the default in Ubuntu 9.04 (the current Ubuntu release).

        Show
        Doug Cutting added a comment - I have boost 1.34.1, the default in Ubuntu 9.04 (the current Ubuntu release).
        Hide
        Scott Banachowski added a comment -

        This compile error is the same one Thiru found, and it's related to older boost (I am using 1.38).
        Doug, can you confirm if your boost version is < 1.38. possibly 1.33?

        Show
        Scott Banachowski added a comment - This compile error is the same one Thiru found, and it's related to older boost (I am using 1.38). Doug, can you confirm if your boost version is < 1.38. possibly 1.33?
        Hide
        Doug Cutting added a comment -

        When I try to compile after applying this, I get:

        g++ -Wall -Werror -g -I./api -I./parser -I. -isystem/usr/include/boost -c -o obj/NodeImpl.o impl/NodeImpl.cc
        impl/NodeImpl.cc: In function 'avro::NodePtr avro::nodeFromCompilerNode(avro::CompilerNode&)':
        impl/NodeImpl.cc:160: error: 'class avro::NodePtr' has no member named 'reset'
        impl/NodeImpl.cc:166: error: 'class avro::NodePtr' has no member named 'reset'
        impl/NodeImpl.cc:170: error: 'class avro::NodePtr' has no member named 'reset'
        impl/NodeImpl.cc:174: error: 'class avro::NodePtr' has no member named 'reset'
        impl/NodeImpl.cc:178: error: 'class avro::NodePtr' has no member named 'reset'
        impl/NodeImpl.cc:182: error: 'class avro::NodePtr' has no member named 'reset'
        impl/NodeImpl.cc:186: error: 'class avro::NodePtr' has no member named 'reset'
        impl/NodeImpl.cc:190: error: 'class avro::NodePtr' has no member named 'reset'
        make: *** [obj/NodeImpl.o] Error 1

        Show
        Doug Cutting added a comment - When I try to compile after applying this, I get: g++ -Wall -Werror -g -I./api -I./parser -I. -isystem/usr/include/boost -c -o obj/NodeImpl.o impl/NodeImpl.cc impl/NodeImpl.cc: In function 'avro::NodePtr avro::nodeFromCompilerNode(avro::CompilerNode&)': impl/NodeImpl.cc:160: error: 'class avro::NodePtr' has no member named 'reset' impl/NodeImpl.cc:166: error: 'class avro::NodePtr' has no member named 'reset' impl/NodeImpl.cc:170: error: 'class avro::NodePtr' has no member named 'reset' impl/NodeImpl.cc:174: error: 'class avro::NodePtr' has no member named 'reset' impl/NodeImpl.cc:178: error: 'class avro::NodePtr' has no member named 'reset' impl/NodeImpl.cc:182: error: 'class avro::NodePtr' has no member named 'reset' impl/NodeImpl.cc:186: error: 'class avro::NodePtr' has no member named 'reset' impl/NodeImpl.cc:190: error: 'class avro::NodePtr' has no member named 'reset' make: *** [obj/NodeImpl.o] Error 1
        Hide
        Scott Banachowski added a comment -

        Thiru found it works with recent boost versions, but does not compile with older versions (due to boost API change).
        I'll fix this patch to work with earlier boost versions.

        Show
        Scott Banachowski added a comment - Thiru found it works with recent boost versions, but does not compile with older versions (due to boost API change). I'll fix this patch to work with earlier boost versions.
        Hide
        Scott Banachowski added a comment -

        I've made some changes to the parser. This allows the order of attributes in a JSON schema declaration to appear in any order, and allows for metadata attributes to appear in records and field definitions.

        There was one source file added, and a couple of test schemas, so I added a script that applies the patch and does svn add commands on the new files.

        After running, svn status -q reports:

        M src/c++/test/testparser.cc
        M src/c++/test/precompile.cc
        M src/c++/impl/ValidSchema.cc
        M src/c++/impl/NodeImpl.cc
        M src/c++/impl/Compiler.cc
        M src/c++/api/NodeConcepts.hh
        M src/c++/api/NodeImpl.hh
        M src/c++/api/Compiler.hh
        A src/c++/api/CompilerNode.hh
        M src/c++/parser/avro.l
        M src/c++/parser/avro.y
        M src/c++/jsonschemas/record2
        A src/c++/jsonschemas/verboseint
        M src/c++/jsonschemas/enum
        A src/c++/jsonschemas/int
        A src/c++/jsonschemas/map
        M src/c++/jsonschemas/array
        M src/c++/jsonschemas/record

        Show
        Scott Banachowski added a comment - I've made some changes to the parser. This allows the order of attributes in a JSON schema declaration to appear in any order, and allows for metadata attributes to appear in records and field definitions. There was one source file added, and a couple of test schemas, so I added a script that applies the patch and does svn add commands on the new files. After running, svn status -q reports: M src/c++/test/testparser.cc M src/c++/test/precompile.cc M src/c++/impl/ValidSchema.cc M src/c++/impl/NodeImpl.cc M src/c++/impl/Compiler.cc M src/c++/api/NodeConcepts.hh M src/c++/api/NodeImpl.hh M src/c++/api/Compiler.hh A src/c++/api/CompilerNode.hh M src/c++/parser/avro.l M src/c++/parser/avro.y M src/c++/jsonschemas/record2 A src/c++/jsonschemas/verboseint M src/c++/jsonschemas/enum A src/c++/jsonschemas/int A src/c++/jsonschemas/map M src/c++/jsonschemas/array M src/c++/jsonschemas/record
        Scott Banachowski made changes -
        Attachment applypatch98.sh [ 12418292 ]
        Hide
        Scott Banachowski added a comment -

        script to apply patch and run svn add commands on new files

        Show
        Scott Banachowski added a comment - script to apply patch and run svn add commands on new files
        Scott Banachowski made changes -
        Attachment AVRO-98.patch [ 12418291 ]
        Hide
        Scott Banachowski added a comment -

        Patch with code changes

        Show
        Scott Banachowski added a comment - Patch with code changes
        Scott Banachowski made changes -
        Field Original Value New Value
        Component/s c++ [ 12312882 ]
        Scott Banachowski created issue -

          People

          • Assignee:
            Scott Banachowski
            Reporter:
            Scott Banachowski
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 168h
              168h
              Remaining:
              Remaining Estimate - 168h
              168h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development