How to fix an awful marshal reflection hack

[Mark]

I have this code which works but has a horrible hack:

...

nullable := false

kind := field.Kind() // field's type is reflect.Value
if kind == reflect.Ptr {
    // FIXME How can I improve upon this truly awful hack?
    switch field.Type().String() {
    case "*int", "*int8", "*uint8", "*int16", "*uint16", "*int32", "*uint32", "*int64", "*uint64":
        kind = reflect.Int
    case "*float32", "*float64":
        kind = reflect.Float64
    }
    nullable = true
}
switch kind {
case reflect.Bool:
    out.WriteString("bool")
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64:
    out.WriteString("int")
case reflect.Float32, reflect.Float64:
    out.WriteString("real")
...

if nullable {

    out.WriteByte('?')

}

What is the correct way to achieve what I'm aiming for?

[burak serdar]

On Wed, Nov 30, 2022 at 5:29 AM 'Mark' via golang-nuts <golan...@googlegroups.com> wrote:

I have this code which works but has a horrible hack:

...

nullable := false

kind := field.Kind() // field's type is reflect.Value
if kind == reflect.Ptr {

This should work:

kind = field.Elem().Kind()

 

    // FIXME How can I improve upon this truly awful hack?
    switch field.Type().String() {
    case "*int", "*int8", "*uint8", "*int16", "*uint16", "*int32", "*uint32", "*int64", "*uint64":
        kind = reflect.Int
    case "*float32", "*float64":
        kind = reflect.Float64
    }
    nullable = true
}
switch kind {
case reflect.Bool:
    out.WriteString("bool")
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64:
    out.WriteString("int")
case reflect.Float32, reflect.Float64:
    out.WriteString("real")
...

if nullable {

    out.WriteByte('?')

}

What is the correct way to achieve what I'm aiming for?

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/5ff4b6f6-405c-4ca5-9299-7c15e1d5c424n%40googlegroups.com.

[Mark]

Yes, I'd already tried that (that's what I started with) and unfortunately it doesn't work.

[burak serdar]

On Wed, Nov 30, 2022 at 10:17 AM 'Mark' via golang-nuts <golan...@googlegroups.com> wrote:

Yes, I'd already tried that (that's what I started with) and unfortunately it doesn't work.

It fails if field.Elem() is nil. Try this:

  kind = field.Type().Elem().Kind()

 

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/c2d154f2-b425-4cc9-a015-af30f4dc9de2n%40googlegroups.com.

[Mark]

Thanks. I've now tried that as follows:

        fmt.Printf("@@@@@@: %T %v\n", field, field)
        kind = field.Type().Elem().Kind()
        fmt.Printf("######: %T %v\n", field, field)

In every case the output for kind before and after was identical. (Naturally, I tried without the print statements too.) And, of course the tests fail. So I'm _still_ using the awful hack!

[Dan Kortschak]

On Thu, 2022-12-01 at 00:33 -0800, 'Mark' via golang-nuts wrote:
> Thanks. I've now tried that as follows:
>
>         fmt.Printf("@@@@@@: %T %v\n", field, field)
>         kind = field.Type().Elem().Kind()
>         fmt.Printf("######: %T %v\n", field, field)
>
> In every case the output for kind before and after was identical.
> (Naturally, I tried without the print statements too.) And, of course
> the tests fail. So I'm _still_ using the awful hack!
>

Doesn't this do what you want?

https://go.dev/play/p/7jUw_iW8B_8

[Mark]

I tried that and it works in the playground, and I added more types and it still works in the playground.

But in my program it still doesn't work:-(

The actual code is here tdb-go in the file marshal.go from line 133 function marshalTableMetaData().

If you run: go test it all works; but if you replace the call to hack() and use nullable as you did in the playground, some of the tests fail.

[Marvin Renich]

* 'Mark' via golang-nuts <golan...@googlegroups.com> [221201 05:17]:

> I tried that and it works in the playground, and I added more types and it

> still works in the playground <https://go.dev/play/p/Yxzj4tAAGhM>.

> But in my program it still doesn't work:-(

> The actual code is here tdb-go <https://github.com/mark-summerfield/tdb-go>

> in the file marshal.go from line 133 function marshalTableMetaData().
> If you run: go test it all works; but if you replace the call to hack() and
> use nullable as you did in the playground, some of the tests fail.

You don't show the code that doesn't work (i.e. with nullable). Did you
make a typo like you did in your code below?

> On Thursday, December 1, 2022 at 9:45:48 AM UTC kortschak wrote:
>
> > On Thu, 2022-12-01 at 00:33 -0800, 'Mark' via golang-nuts wrote:
> > > Thanks. I've now tried that as follows:
> > >
> > > fmt.Printf("@@@@@@: %T %v\n", field, field)
> > > kind = field.Type().Elem().Kind()
> > > fmt.Printf("######: %T %v\n", field, field)

Note that in both Printf statements, you are using field rather than
kind. If the two Printf's gave different results, I would consider it a
compiler bug (a really egregious one!).

> > > In every case the output for kind before and after was identical.
> > > (Naturally, I tried without the print statements too.) And, of course
> > > the tests fail. So I'm _still_ using the awful hack!
> > >
> >
> > Doesn't this do what you want?
> >
> > https://go.dev/play/p/7jUw_iW8B_8

...Marvin

[Mark]

The reason there's no nullable in the real code is that it isn't needed there: if the field is to a pointer variable (e.g., *string), then I call hack() and that adds the '?' to the string so no need for a nullable bool; otherwise for non-pointers it falls through to the normal processing. So the complete -- and working -- code is in the repo and go test works. But replacing the call to hack() with kind = field.Type().Elem().Kind() breaks the tests.

[burak serdar]

On Thu, Dec 1, 2022 at 6:39 AM 'Mark' via golang-nuts <golan...@googlegroups.com> wrote:

The reason there's no nullable in the real code is that it isn't needed there: if the field is to a pointer variable (e.g., *string), then I call hack() and that adds the '?' to the string so no need for a nullable bool; otherwise for non-pointers it falls through to the normal processing. So the complete -- and working -- code is in the repo and go test works. But replacing the call to hack() with kind = field.Type().Elem().Kind() breaks the tests.

Your original code set the nullable to true in the if-block. Do you still have that piece?

 

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/e77369e7-7387-496e-ab02-0f47d5319fd6n%40googlegroups.com.

[Mark]

No, because it isn't needed. A field is nullable if it is a pointer (nil equating to null). If the field is a pointer then hack() is called and this adds the '?' to indicate the field is nullable; otherwise the normal code runs.

Essentially, what the code is doing is reading structs which it has no prior knowledge of beyond the fact that the struct has one or more public fields and each of these is a slice of structs and these inner structs have public fields which are always integers, floats, strings, []byte, bool, or time.Time values -- or are pointers to one of these.

So the marshal function has to output the corresponding Tdb typename, e.g., 'bytes' for a Go []byte field and 'bytes?' for a Go *[]byte field, and so on.

Right now the code works fine for non-pointer types, but for pointers, I call the hack function -- which also works fine, but as the name suggests seems to me like it is doing the right thing the wrong way.

[Mark]

Thanks for all your ideas. I've finally solved it. The complete code is in the repo, but below is the essence. The key mistake I'd made was not accounting correctly for a slice of pointers to slices, e.g., []*[]byte.

nullable := false
kind := field.Kind()

if kind == reflect.Ptr {
    kind = field.Type().Elem().Kind()

    nullable = true
}
switch kind {
case reflect.Bool:
    out.WriteString("bool")
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64:
    out.WriteString("int")
case reflect.Float32, reflect.Float64:
    out.WriteString("real")

case reflect.String:
    out.WriteString("str")
case reflect.Slice:
    if field.Kind() == reflect.Ptr && field.Elem().Type() == byteSliceType {
        out.WriteString("bytes")
    } else {
        x := field.Interface()
        if reflect.TypeOf(x) == byteSliceType {
            out.WriteString("bytes")
        } else {
            return isDate, fmt.Errorf(...